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

[David Goulet]

Hi Alex,

Comments below.

On 11-12-05 10:11 AM, Alexandre Montplaisir 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 |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/lttng-sessiond/main.c b/lttng-sessiond/main.c
> index fcae023..aa15554 100644
> --- a/lttng-sessiond/main.c
> +++ b/lttng-sessiond/main.c
> @@ -58,6 +58,7 @@
>  #include "utils.h"
>  
>  #define CONSUMERD_FILE	"lttng-consumerd"
> +#define MAX_LENGTH	255
>  
>  struct consumer_data {
>  	enum lttng_consumer_type type;
> @@ -183,7 +184,7 @@ static const char *consumerd64_libdir =
>  static
>  void setup_consumerd_path(void)
>  {
> -	const char *path, *libdir;
> +	char *path, *libdir;
>  
>  	/*
>  	 * Allow INSTALL_BIN_PATH to be used as a target path for the
> @@ -213,17 +214,17 @@ void setup_consumerd_path(void)
>  	 */
>  	path = getenv("LTTNG_CONSUMERD32_PATH");
>  	if (path) {
> -		consumerd32_path = path;
> +		consumerd32_path = strncat(path, "/" CONSUMERD_FILE, MAX_LENGTH);

Three things here.

First, you *CAN NOT* overwrite path since it is a pointer to the value in the
environment. The caller must not modify this.

Second, the libc already define standard length like NAME_MAX or PATH_MAX. It
would be better to use them and not define any other constant.

Finally, I'll recommend here you use asprintf() (or any printf family functions)
to create the consumerd32_path and one reason is to use POSIX functions.

Same for the next modifications.

Cheers!
David

>  	}
>  	path = getenv("LTTNG_CONSUMERD64_PATH");
>  	if (path) {
> -		consumerd64_path = path;
> +		consumerd64_path = strncat(path, "/" CONSUMERD_FILE, MAX_LENGTH);
>  	}
> -	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;
>  	}