[ltt-dev] [UST PATCH 3/5] Restructure the ustctl_get_online_pids command

[Mathieu Desnoyers]

* Nils Carlson (nils.carlson at ericsson.com) wrote:
> Restructure the command to separate the pid gathering.
> This will allow a root user to gather pid from multiple user
> directories.
> 
> Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
> ---
>  libustctl/libustctl.c |   80 ++++++++++++++++++++++++++++++------------------
>  1 files changed, 50 insertions(+), 30 deletions(-)
> 
> diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
> index d57e645..2453a99 100644
> --- a/libustctl/libustctl.c
> +++ b/libustctl/libustctl.c
> @@ -88,53 +88,73 @@ int ustctl_connect_pid(pid_t pid)
>  	return sock;
>  }
>  
> -pid_t *ustctl_get_online_pids(void)
> +static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
> +			    unsigned int *pid_list_size)
>  {
>  	struct dirent *dirent;
> -	DIR *dir;
> -	unsigned int ret_size = 1 * sizeof(pid_t), i = 0;

Instead of using "sizeof(pid_t)" for ret_size base unit, we could just
keep the array length, and multiply by sizeof(pid_t) when needed. This
is what is done pretty much everywhere else in the lttng code.

> -
> -	dir = opendir(SOCK_DIR);
> -	if (!dir) {
> -		return NULL;
> -	}
> -
> -	pid_t *ret = (pid_t *) malloc(ret_size);
> +	int i = 0;
>  
>  	while ((dirent = readdir(dir))) {
>  		if (!strcmp(dirent->d_name, ".") ||
> -		    !strcmp(dirent->d_name, "..")) {
> +		    !strcmp(dirent->d_name, "..") ||
> +		    !strcmp(dirent->d_name, "ust-consumer") ||
> +		    dirent->d_type == DT_DIR) {
>  
>  			continue;
>  		}
>  
> -		if (dirent->d_type != DT_DIR &&
> -		    !!strcmp(dirent->d_name, "ust-consumer")) {
> -
> -			sscanf(dirent->d_name, "%u", (unsigned int *) &ret[i]);
> -			/* FIXME: Here we previously called pid_is_online, which
> -			 * always returned 1, now I replaced it with just 1.
> -			 * We need to figure out an intelligent way of solving
> -			 * this, maybe connect-disconnect.
> -			 */
> -			if (1) {
> -				ret_size += sizeof(pid_t);
> -				ret = (pid_t *) realloc(ret, ret_size);
> -				++i;
> -			}
> +		sscanf(dirent->d_name, "%u",
> +		       (unsigned int *) &(*pid_list)[i]);

I think there is a bug with this cast if the pid_list target is ever
expected to be a 64-bit unsigned long. I would recommend putting the
result in a local variable instead, and then copying it to the pid_list
element.

> +		/* FIXME: Here we previously called pid_is_online, which
> +		 * always returned 1, now I replaced it with just 1.
> +		 * We need to figure out an intelligent way of solving
> +		 * this, maybe connect-disconnect.
> +		 */

Yep, trying to "ping" the application through its listener socket seems
appropriate. This will let us make sure we have the appropriate
credentials to talk to the application (the trace control app is either
the same user, or in the "tracing" group, or "root").

Thanks,

Mathieu

> +		if (1) {
> +			*pid_list_size += sizeof(pid_t);
> +			*pid_list = (pid_t *)realloc(*pid_list, *pid_list_size);
> +			i++;
>  		}
>  	}
>  
> -	ret[i] = 0; /* Array end */
> +	(*pid_list)[i] = 0; /* Array end */
> +}
>  
> -	if (ret[0] == 0) {
> -		/* No PID at all */
> -		free(ret);
> +pid_t *ustctl_get_online_pids(void)
> +{
> +	char *dir_name;
> +	DIR *dir;
> +	unsigned int pid_list_size = sizeof(pid_t);
> +	pid_t *pid_list = NULL;
> +
> +	dir_name = ustcomm_user_sock_dir();
> +	if (!dir_name) {
>  		return NULL;
>  	}
>  
> +	dir = opendir(dir_name);
> +	if (!dir) {
> +		goto free_dir_name;
> +	}
> +
> +	pid_list = (pid_t *) malloc(pid_list_size);
> +
> +	get_pids_in_dir(dir, &pid_list, &pid_list_size);
> +
> +	if (pid_list[0] == 0) {
> +		/* No PID at all */
> +		free(pid_list);
> +		pid_list = NULL;
> +		goto close_dir;
> +	}
> +
> +close_dir:
>  	closedir(dir);
> -	return ret;
> +
> +free_dir_name:
> +	free(dir_name);
> +
> +	return pid_list;
>  }
>  
>  /**
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

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