Re: [RFC] Refactor elf_prpsinfo{32,64} structures

[Pedro Alves <palves at redhat dot com>]

On 16-11-2012 04:18, Sergio Durigan Junior wrote:

> +/* Unsigned 64-bit integer aligned to 8 bytes.  */
> +typedef uint64_t __attribute__ ((__aligned__ (8))) a8_uint64_t;

So this has been moved to what should be host-independent code;
not sure using attribute aligned is kosher.

> +#define PRPSINFO64_FIELD_SIZE(field) sizeof (((prpsinfo64_t *) 0)->field)
> +
> +struct elf_prpsinfo64
> +  {
> +    int8_t pr_state;			/* Numeric process state.  */
> +    char pr_sname;			/* Char for pr_state.  */
> +    int8_t pr_zomb;			/* Zombie.  */
> +    int8_t pr_nice;			/* Nice val.  */
> +    a8_uint64_t pr_flag;		/* Flags.  */
> +    uint32_t pr_uid;
> +    uint32_t pr_gid;
> +    int32_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
> +    /* Lots missing */
> +    char pr_fname[16];			/* Filename of executable.  */
> +    char pr_psargs[ELF_PRARGSZ];	/* Initial part of arg list.  */
> +  };
> +
> +/* Process status and info.  In the end we do provide typedefs for them.  */
> +typedef struct elf_prpsinfo32 prpsinfo32_t;
> +typedef struct elf_prpsinfo64 prpsinfo64_t;

So these are the external representations of these types, right?

The BFD way to handle this is I think to define the external types's
fields as char arrays.  So you'd have something like:

struct elf_external_prpsinfo64
  {
    char pr_state;			/* Numeric process state.  */
    char pr_sname;			/* Char for pr_state.  */
    char pr_zomb;			/* Zombie.  */
    char pr_nice;			/* Nice val.  */
    char gap[4];                        /* alignment gap */
    char pr_flag[8];		/* Flags.  */
    char pr_uid[8];
    char pr_gid[8];
    char pr_pid[4];
    char pr_ppid[4];
    char pr_pgrp[4];
    char pr_sid[4];
    /* Lots missing */
    char pr_fname[16];			/* Filename of executable.  */
    char pr_psargs[ELF_PRARGSZ];	/* Initial part of arg list.  */
  } ATTRIBUTE_PACKED;

Dunno if the gap is always present on all targets or not.  See below.

> +static char *
> +elf_i386_write_core_note (bfd *abfd, char *buf, int *bufsiz,
> +			  int note_type, ...)
> +{
> +  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
> +  va_list ap;
> +  const struct elf_internal_prpsinfo *prpsinfo;
> +  long pid;
> +  int cursig;
> +  const void *gregs;
> +  char data[sizeof (prpsinfo32_t)];
> +  char *p = data;
> +
> +  switch (note_type)
> +    {
> +    default:
> +      return NULL;
> +
> +    case NT_PRPSINFO:
> +      va_start (ap, note_type);
> +      prpsinfo = va_arg (ap, const struct elf_internal_prpsinfo *);
> +      va_end (ap);
> +
> +      memset (data, 0, sizeof (data));
> +
> +      memcpy (p, &prpsinfo->pr_state, PRPSINFO32_FIELD_SIZE (pr_state));
> +      p += PRPSINFO32_FIELD_SIZE (pr_state);

here this doesn't handle the case of different host and target endianness.
I suggest looking at how this is usually handled in the swapping routines,
like e.g., elf_swap_ehdr_out (note how that's actually "templated" code - don't
know if it makes sense to do something similar for these structures).
Going the swap way, you'd write something like

  H_PUT_32 (abfd, prpsinfo->pr_uid, dst->pr_uid);

etc. which handles the memcpy+endianness in one go, and you don't have
to care here for advancing a pointer, and caring for the alignment gap
in 64-bit here, since that'd be embedded in the external struct itself instead.

But anyway, at a minimum, if this stays as you have it, it this should
handle the endianness of each field.

Thanks!

-- 
Pedro Alves