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

[Mathieu Desnoyers]

* Nils Carlson (nils.carlson at ericsson.com) wrote:
> Changes since v1:
> 	* Use list length, not the size in bytes
> 	* Fix a possible bug for pid_t in a sscanf
> 	* Use list length in the dir scanning function
> 
> 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 |   81 +++++++++++++++++++++++++++++++------------------
>  1 files changed, 51 insertions(+), 30 deletions(-)
> 
> diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
> index d57e645..5625f43 100644
> --- a/libustctl/libustctl.c
> +++ b/libustctl/libustctl.c
> @@ -88,53 +88,74 @@ 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;
> -
> -	dir = opendir(SOCK_DIR);
> -	if (!dir) {
> -		return NULL;
> -	}
> -
> -	pid_t *ret = (pid_t *) malloc(ret_size);
> +	unsigned int read_pid;
>  
>  	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", &read_pid);
> +
> +		(*pid_list)[*pid_list_size - 1] = read_pid;
> +		/* 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) {
> +			(*pid_list_size)++;
> +			*pid_list = realloc(*pid_list,
> +					    *pid_list_size * sizeof(pid_t));

Rather than reallocating the memory space each time we add a single
entry, I would recommend to double the size each time we need to grow
the allocated space. We use a "0" entry at the end to denote the end of
the list anyway, so allocating an area larger than needed should not
hurt, and it will turn this O(nb items) * mem allocation runtime cost
into a O(log(nb items)) * mem allocation runtime cost.

Thanks,

Mathieu

>  		}
>  	}
>  
> -	ret[i] = 0; /* Array end */
> +	(*pid_list)[*pid_list_size - 1] = 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 = 1;
> +	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 = malloc(pid_list_size * sizeof(pid_t));
> +
> +	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