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

[Nils Carlson]

On Mar 31, 2011, at 9:48 PM, Mathieu Desnoyers wrote:

> * 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.

Yepp, can look at it. This just follows the pattern of the old code.  
But nothing to say the old code was doing it right.

/Nils

>
> 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
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev