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

[David Goulet]

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.

Thanks
David

> Thanks,
> 
> Mathieu
> 
>> +		if (ret < 0) {
>> +			PERROR("asprintf consumerd32_path");
>> +			goto error;
>> +		}
>>  	}
>>  	path = getenv("LTTNG_CONSUMERD64_PATH");
>>  	if (path) {
>> -		consumerd64_path = path;
>> +		ret = asprintf((char**)&consumerd64_path, "%s/" CONSUMERD_FILE, path);
>> +		if (ret < 0) {
>> +			PERROR("asprintf consumerd64_path");
>> +			goto error;
>> +		}
>>  	}
>> -	libdir = getenv("LTTNG_TOOLS_CONSUMERD32_LIBDIR");
>> +	libdir = getenv("LTTNG_CONSUMERD32_LIBDIR");
>>  	if (libdir) {
>> -		consumerd32_libdir = libdir;
>> +		consumerd32_libdir = strdup(libdir);
>> +		if (consumerd32_libdir == NULL) {
>> +			PERROR("strdup consumerd32_libdir");
>> +			goto error;
>> +		}
>>  	}
>> -	libdir = getenv("LTTNG_TOOLS_CONSUMERD64_LIBDIR");
>> +	libdir = getenv("LTTNG_CONSUMERD64_LIBDIR");
>>  	if (libdir) {
>> -		consumerd64_libdir = libdir;
>> +		consumerd64_libdir = strdup(libdir);
>> +		if (consumerd64_libdir == NULL) {
>> +			PERROR("strdup consumerd64_libdir");
>> +			goto error;
>> +		}
>>  	}
>> +
>> +error:
>> +	return;
>>  }
>>  
>>  /*
>> -- 
>> 1.7.7.3
>>
>>
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev at lists.lttng.org
>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>
>