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

[Mathieu Desnoyers]

* 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, ...)
> 
> Other changes:
> Renamed the _LIBDIR env vars for consitency.
> Added error handling to setup_consumerd_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..d43ccdc 100644
> --- a/lttng-sessiond/main.c
> +++ b/lttng-sessiond/main.c
> @@ -181,9 +181,11 @@ static const char *consumerd64_libdir =
>  	__stringify(CONFIG_CONSUMERD64_LIBDIR);
>  
>  static
> -void setup_consumerd_path(void)
> +int setup_consumerd_path(void)
>  {
>  	const char *path, *libdir;
> +	char *tmp_path;
> +	int ret;
>  
>  	/*
>  	 * Allow INSTALL_BIN_PATH to be used as a target path for the
> @@ -213,20 +215,35 @@ void setup_consumerd_path(void)
>  	 */
>  	path = getenv("LTTNG_CONSUMERD32_PATH");
>  	if (path) {
> -		consumerd32_path = path;
> +		ret = asprintf(&tmp_path, "%s/" CONSUMERD_FILE, path);
> +		if (ret < 0) {
> +			PERROR("asprintf consumerd32_path");
> +			goto error;
> +		}
> +		consumerd32_path = tmp_path;

Sorry for the few rounds, after looking at the results, please change
the existing consumerd32_path for:

static char consumerd32_path[PATH_MAX] =
	__stringify(CONFIG_CONSUMERD32_PATH);;

And then use snprintf to update it.

asnprintf here will trigger a memory leak that valgrind would complain
about.

Same for 64-bit.

Thanks,

Mathieu


>  	}
>  	path = getenv("LTTNG_CONSUMERD64_PATH");
>  	if (path) {
> -		consumerd64_path = path;
> +		ret = asprintf(&tmp_path, "%s/" CONSUMERD_FILE, path);
> +		if (ret < 0) {
> +			PERROR("asprintf consumerd64_path");
> +			goto error;
> +		}
> +		consumerd64_path = tmp_path;
>  	}
> -	libdir = getenv("LTTNG_TOOLS_CONSUMERD32_LIBDIR");
> +	libdir = getenv("LTTNG_CONSUMERD32_LIBDIR");
>  	if (libdir) {
>  		consumerd32_libdir = libdir;
>  	}
> -	libdir = getenv("LTTNG_TOOLS_CONSUMERD64_LIBDIR");
> +	libdir = getenv("LTTNG_CONSUMERD64_LIBDIR");
>  	if (libdir) {
>  		consumerd64_libdir = libdir;
>  	}
> +
> +	return 0;
> +
> +error:
> +	return ret;
>  }
>  
>  /*
> @@ -4172,7 +4189,10 @@ int main(int argc, char **argv)
>  		goto error;
>  	}
>  
> -	setup_consumerd_path();
> +	/* Setup the paths for the consumer daemon and libraries */
> +	if ((ret = setup_consumerd_path()) < 0) {
> +		goto error;
> +	}
>  
>  	/* Parse arguments */
>  	progname = argv[0];
> -- 
> 1.7.7.3
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com