Re: [PATCH/RFC 01/02 v2] Refactor PRPSINFO handling on Binutils

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Thursday, January 03 2013, Jan Kratochvil wrote:

> On Thu, 03 Jan 2013 00:32:12 +0100, Sergio Durigan Junior wrote:
>> On Tuesday, January 01 2013, Jan Kratochvil wrote:
>> > On Sun, 30 Dec 2012 02:49:36 +0100, Sergio Durigan Junior wrote:
>> >> +/* Internal structure which holds information to be included in the
>> >> +   PRPSINFO section of the corefile.
>> >> +
>> >> +   This is an "internal" structure in the sense that it should be used to
>> >> +   pass information to BFD (via the `elfcore_write_prpsinfo', for example),
>> >> +   so things like endianess shouldn't be an issue.  This structure will
>> >> +   eventually be converted in one of the `elf_external_*' structures
>> >> +   below.  */
>> >> +
>> >> +struct elf_internal_prpsinfo
>> >> +  {
>> >> +    char pr_state;			/* Numeric process state.  */
>> >> +    char pr_sname;			/* Char for pr_state.  */
>> >> +    char pr_zomb;			/* Zombie.  */
>> >> +    char pr_nice;			/* Nice val.  */
>> >> +    unsigned long pr_flag;		/* Flags.  */
>> >> +    unsigned int pr_uid;
>> >> +    unsigned int pr_gid;
>> >> +    int pr_pid, pr_ppid, pr_pgrp, pr_sid;
>> >> +    /* Lots missing */
>> >
>> > This comment seems off-topic here.  It fully represents the core file
>> > structure.
>> 
>> I can remove the comment, but it doesn't say that the structure does not
>> represent a corefile struct.  It just explains the reason why it is
>> called "internal".
>
> As /usr/include/linux/elfcore.h (=from Linux kernel) contains:
>
> struct elf_prpsinfo
> {
> [...]
>         pid_t   pr_pid, pr_ppid, pr_pgrp, pr_sid;
>         /* Lots missing */
>         char    pr_fname[16];   /* filename of executable */
> [...]
> };
>
> we should look at it from the point of Linux kernel.  It probably made sense
> as Linux kernel could store some more info there.

Ops, sorry, I thought you were talking about the comment above the
structure, and not about the "Lots missing".  Then I agree, it can be
removed.  Sorry about the confusion.  I will send an updated patch in a
few moments.

-- 
Sergio