Re: [PATCH] Add extra 'info os' information types for Linux

Tom Tromey
2011-10-21T20:58:16Z

>>>>> ">" == Kwok Cheung Yeung <kcy@codesourcery.com> writes:

>> This patch greatly extends the OS information types available to the
>> user using the 'info os' command in Linux. Since the OS info backend
>> is now unified, this information can be obtained from both local and
>> remote (gdbserver) targets.

I think the idea is good.

>> +static int
>> +compare_processes (const void *process1, const void *process2)

Most of the new functions need introductory comments.

>> + static struct buffer buffer;

Wow, another growable buffer type. I didn't know about this one.
Let's see.. VEC, dynstr, obstack, buffer... can we have a 5th? :)

>> + pid_t *process_list = (pid_t *) xmalloc (list_block_size * 2 * sizeof (pid_t));

Too bad VEC isn't available here.

>> + char procentry[sizeof ("/proc/4294967295")];

One occasionally hears talk of 64 bit PIDs.

>> +#if 0
>> + struct stat statbuf;
>> + stat (dp2->d_name, &statbuf);
>> +#endif

No new #if 0 code.

>> +static void
>> +time_from_int (char *time, int maxlen, int seconds)

Why int and not just time_t?
...

>> + items_read = sscanf (buf,
>> + "%d %d %o %d %d %d %d %u %u %u %u %d %d %d",
>> + &key, &shmid, &perms, &size,
>> + &cpid, &lpid,
>> + &nattch,
>> + &uid, &gid, &cuid, &cgid,
>> + &atime, &dtime, &ctime);
[...]
>> + char atime_str[32], dtime_str[32], ctime_str[32];
[...]
>> + time_from_int (atime_str, sizeof (atime_str), atime);
>> + time_from_int (dtime_str, sizeof (dtime_str), dtime);
>> + time_from_int (ctime_str, sizeof (ctime_str), ctime);

I think it is probably better to use long at least.

Tom