[ltt-dev] [UST PATCH] Get Online targets checker
Matthew Khouzam
matthew.khouzam at ericsson.com
Mon Mar 28 13:12:20 EDT 2011
Added a check in get online pids to only send the pids that are
currently online.
Now passes checkpactch.pl and
has no obvious style problems and is ready for submission.
Signed-off-by: Matthew Khouzam <matthew.khouzam at ericsson.com>
---
libustctl/libustctl.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
index d57e645..9317440 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,17 @@ 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:50 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 | 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]) ;
> please remove extra space before , and before ; and after snprintf(
>
>> + 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 ) {
> if (proc_dir) {
>
> instead. (ensure white spaces are ok)
>
> You should try running your patch trough the checkpatch.pl program
> available in the Linux kernel scripts/ directory, it will tell you all
> these coding style issues.
>
> Thanks,
>
> Mathieu
>
>> 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
>>>>>>
More information about the lttng-dev
mailing list