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

[Matthew Khouzam]

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 |   15 ++++++++++++++-
  1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
index d57e645..f0b1fb1 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,18 @@ 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);
+             /*
+              * 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 its pid can be still taken when
+              * reading online pids.
+              */
+             if( proc_dir )    {
                  ret_size += sizeof(pid_t);
                  ret = (pid_t *) realloc(ret, ret_size);
                  ++i;
-- 
1.7.0.4

On 11-03-28 11:21 AM, Mathieu Desnoyers wrote:
> * 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
>>>>