libprocstat(3): retrieve process command line args and environment

[Mikolaj Golub]

On Fri, Mar 29, 2013 at 11:22:45AM +0200, Konstantin Belousov wrote:
> On Thu, Mar 28, 2013 at 11:18:21PM +0200, Mikolaj Golub wrote:
> > On Thu, Mar 28, 2013 at 12:51:34PM +0200, Konstantin Belousov wrote:
> > 
> > > In the generic Elf 64bit draft specification I have, the notes sections
> > > are specified to consists of entries, each of which is an array of 8-byte
> > > words. I think we are right using the 8-byte alignment.
> > 
> > I have impression many implementations use 4-byte alignment. E.g. in
> > NetBSD:
> > 
> > sys/kern/core_elf32.c:
> > 
> > #define ELFROUNDSIZE    4       /* XXX Should it be sizeof(Elf_Word)? */
> > #define elfround(x)     roundup((x), ELFROUNDSIZE)
> Note that this is core_elf32. I am concerned with the 64-bit cores.

core_elf64.c:

#define	ELFSIZE	64

#include "core_elf32.c"

> > 
> > Also, we have inconsistency with imgactl_elf.c/parse_notes(), which
> > uses 4-byte alignment:
> > 
> > 		note = (const Elf_Note *)((const char *)(note + 1) +
> > 		    roundup2(note->n_namesz, sizeof(Elf32_Addr)) +
> > 		    roundup2(note->n_descsz, sizeof(Elf32_Addr)));
> > 
> > I suppose there were no issues before, because accidentally the sizes
> > of all notes we had were 8 bytes aligned.
> Indeed, both ABI and NOINIT notes have size which is multiple of 8.
> 
> > 
> > Now, when I add new notes it will break things. I don't have strong
> > opinion, it will be ok for me to leave 8-byte alignment and fix
> > issues, just want to have strong support here :-)
> Well, while the issue is discussed and decided, you could just make
> your new notes size be multiple of 8 too.

I thought about this too. Then I need to be more caerful when
extracting stats from notes, because the length returned by
procstat_core_get() can be more than a real payload.

Ok, I will try this way.

I could add length to the note header, which is currently contains
only structsize, so it would became something like:

struct {
  int structsize;
  int lenght;
}

But not sure it is worth doing, especially if the forced 8-bit
alignment is a temporary mesure.

> > 
> > BTW, looking at NetBSD code I see they set p_align in the note
> > segement to ELFROUNDSIZE:
> > 
> >         /* Write out the PT_NOTE header. */
> >         ws.psections->p_type = PT_NOTE;
> >         ws.psections->p_offset = notestart;
> >         ws.psections->p_vaddr = 0;
> >         ws.psections->p_paddr = 0;
> >         ws.psections->p_filesz = notesize;
> >         ws.psections->p_memsz = 0;
> >         ws.psections->p_flags = PF_R;
> >         ws.psections->p_align = ELFROUNDSIZE;
> > 
> > while we set to 0:
> > 
> > 	/* The note segement. */
> > 	phdr->p_type = PT_NOTE;
> > 	phdr->p_offset = hdrsize;
> > 	phdr->p_vaddr = 0;
> > 	phdr->p_paddr = 0;
> > 	phdr->p_filesz = notesz;
> > 	phdr->p_memsz = 0;
> > 	phdr->p_flags = 0;
> > 	phdr->p_align = 0;
> You mean, for the core dumps ?

yes

> > 
> > Shouldn't we set it to alignment size too? Note also, they set
> > "Segment is readable" flag.
> I think both changes are fine.
> 
> I skimmed over the usermode parts of the patch. One thing I did not liked
> is the mis-handling of the read() return values. If there is short read,
> the errno value is meaningless, but warn() would still append it to
> the message. I suggest to explicitely distinguish -1 and >= 0 returns
> from reads.

ok. Thanks.

-- 
Mikolaj Golub