[lttng-dev] [lttng-tools PATCH v2 1/4] Always add the executable name to consumerd32/64_path

[Alexandre Montplaisir]

On 11-12-05 01:45 PM, Alexandre Montplaisir wrote:
> On 11-12-05 01:41 PM, David Goulet wrote:
>> On 11-12-05 01:33 PM, Mathieu Desnoyers wrote:
>>> * Alexandre Montplaisir (alexandre.montplaisir at gmail.com) wrote:
>>>> The handling of the "consumerd32_path" and "consumerd64_path" was
>>>> lacking consistency ; in some cases it would include the filename
>>>> "lttng-consumerd" at the end, in some others it would not.
>>>>
>>>> What is proposed here is to consider the configure options and
>>>> environment variables as real "paths", so the user would never have
>>>> to specify filenames ("lttng-consumerd" is assumed). However in
>>>> the program itself we'll append the filename, so we can easily
>>>> test for its existence and run exec(consumer_path, ...)
>>>>
>>>> Signed-off-by: Alexandre Montplaisir <alexandre.montplaisir at gmail.com>
>>>> ---
>>>>  lttng-sessiond/main.c |   32 ++++++++++++++++++++++++++------
>>>>  1 files changed, 26 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/lttng-sessiond/main.c b/lttng-sessiond/main.c
>>>> index fcae023..77f86ae 100644
>>>> --- a/lttng-sessiond/main.c
>>>> +++ b/lttng-sessiond/main.c
>>>> @@ -184,6 +184,7 @@ static
>>>>  void setup_consumerd_path(void)
>>>>  {
>>>>  	const char *path, *libdir;
>>>> +	int ret;
>>>>  
>>>>  	/*
>>>>  	 * Allow INSTALL_BIN_PATH to be used as a target path for the
>>>> @@ -213,20 +214,39 @@ void setup_consumerd_path(void)
>>>>  	 */
>>>>  	path = getenv("LTTNG_CONSUMERD32_PATH");
>>>>  	if (path) {
>>>> -		consumerd32_path = path;
>>>> +		ret = asprintf((char**)&consumerd32_path, "%s/" CONSUMERD_FILE, path);
>>> This cast tells me something is wrong. I'll let David reply.
>>>
>> Well... actually, I know it's wrong but the other way around is to use a "tmp"
>> var and after that consumerd32_path = tmp... which is basically the same.
>> However, for the sake of the "uglyness" of cast... I don't know, it does not
>> shock me that much. Mathieu, what's your feeling about that?
>>
>> The important thing here is that consumerd32_path *stays* const since it's
>> initially pointing to a const string.
>>
>> Also Alex, the strdup() is not needed anymore since consumerd32/64... is const
>> anyhow.
> Actually it is, since it doesn't let you assign a const char* to a char*
> directly. Which is why in the initial patch I had dropped the const
> identifier, but as you say it's better to leave those in place.

And I totally had the wrong branch checked out when I tried this. You
are right, both consumerd_path
and the env vars are const char* now, the strdup() isn't need.

(but this will conveniently allow me to test my new mailing list filters...)

>
>> Thanks
>> David
>>
>>