[ltt-dev] [UST PATCH] Get Online targets checker

[Mathieu Desnoyers]

* Matthew Khouzam (matthew.khouzam at ericsson.com) wrote:
> Added a check in get online pids to only send the pids that are  
> currently online.
>
> Signed-off-by: Matthew Khouzam <matthew.khouzam at ericsson.com>
> ---
>  libustctl/libustctl.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
> index d57e645..45cc3b8 100644
> --- a/libustctl/libustctl.c
> +++ b/libustctl/libustctl.c
> @@ -92,6 +92,8 @@ pid_t *ustctl_get_online_pids(void)
>  {
>      struct dirent *dirent;
>      DIR *dir;
> +    DIR *proc_dir;
> +    char proc_dir_path[PATH_MAX];
>      unsigned int ret_size = 1 * sizeof(pid_t), i = 0;
>
>      dir = opendir(SOCK_DIR);
> @@ -117,7 +119,16 @@ pid_t *ustctl_get_online_pids(void)
>               * We need to figure out an intelligent way of solving
>               * this, maybe connect-disconnect.
>               */
> -            if (1) {
> +             snprintf( proc_dir_path, PATH_MAX, "/proc/%u/" , ret[i]) ;
> +             proc_dir = opendir(proc_dir_path);

Small nits:

Please use:

/*
 * some text.
 * other text.
 */

For multiline comments.

> +             /* Note:
> +              * maybe we should remove the empty dir in this pass too.
> +              * Since the detection method is not perfect, this step
> +              * is not yet implemented.
> +              * A process can die, and it's pid can be still taken when

it's -> its.

> +              * reading online pids. */
> +             if( proc_dir != NULL )

 if (proc_dir != NULL) {

or even better:

 if (proc_dir) {

Thanks!

Mathieu



> +            {
>                  ret_size += sizeof(pid_t);
>                  ret = (pid_t *) realloc(ret, ret_size);
>                  ++i;
> -- 
> 1.7.0.4
>
> On 11-03-25 11:59 PM, Mathieu Desnoyers wrote:
>> * Matthew Khouzam (matthew.khouzam at ericsson.com) wrote:
>>> Signed-off-by: Matthew Khouzam<matthew.khouzam at ericsson.com>
>>> ---
>>>   libustctl/libustctl.c |    9 ++++++++-
>>>   1 files changed, 8 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
>>> index d57e645..3b5ae84 100644
>>> --- a/libustctl/libustctl.c
>>> +++ b/libustctl/libustctl.c
>>> @@ -92,8 +92,11 @@ pid_t *ustctl_get_online_pids(void)
>>>   {
>>>       struct dirent *dirent;
>>>       DIR *dir;
>>> +    DIR *proc_dir;
>>> +    char proc_dir_path[80]; /* proc + / + int always smaller than 80 */
>> Hi Matthew,
>>
>> Please don't use hardcoded values like this. It's begging for buffer
>> overflows. You should use PATH_MAX instead.
>>
>>>       unsigned int ret_size = 1 * sizeof(pid_t), i = 0;
>>>       dir = opendir(SOCK_DIR);
>>>       if (!dir) {
>>>   @@ -117,7 +120,11 @@ pid_t *ustctl_get_online_pids(void)
>>>                * We need to figure out an intelligent way of solving
>>>                * this, maybe connect-disconnect.
>>>                */
>>> -            if (1) {
>>> +             snprintf( proc_dir_path, 80, "/proc/%u/" , ret[i]) ;
>> Please use PATH_MAX here too.
>>
>> And if you ever really need to use something smaller (here we really
>> just don't care about a slightly larger stack size in userspace), you
>> should do a define at the top of the file so we know there is a
>> non-standard value buried in there.
>>
>> Thanks,
>>
>> Mathieu
>>
>>
>>> +             proc_dir = opendir(proc_dir_path);
>>> +             /* maybe we should remove the empty dir in this pass too*/
>>> +             if( proc_dir != NULL )
>>> +            {
>>>                   ret_size += sizeof(pid_t);
>>>                   ret = (pid_t *) realloc(ret, ret_size);
>>>                   ++i;
>>> -- 
>>> 1.7.0.4
>>>
>>>
>>> _______________________________________________
>>> 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