Re: [PATCH] auxv support via xfer_partial, for core files and /proc

[Roland McGrath <roland@redhat.com>]

> Roland, FYI, there is already this patch:
> http://sources.redhat.com/ml/gdb-patches/2003-11/msg00204.html

Thanks for the pointer.  I have some comments below comparing the two patches.

> What you should definitly do is add yourself to the write-after-approval 
> list (don't forget to the ChangeLog and to post the committed change).

Done.


Here are the notable differences between the two patches.


1. My patch includes the changes affecting gcore:

	* procfs.c (procfs_make_note_section): If we can read
	TARGET_OBJECT_AUXV data, add an NT_AUXV note containing it.
	* linux-proc.c (linux_make_note_section): Likewise.

   This is not something your patch tries to address.  I think we want those
   changes regardless of the implementation details for TARGET_OBJECT_AUXV.

2. Your patch defines the "info auxv" command for pretty-printing.
   That is a nice thing to have.  However, I would redo your implementation.
   As you've written it, `read_auxv_entry' will be called for each element
   of the vector and read the whole thing fresh each time.  On my 2.6
   kernel, that is 17 times open+pread+close instead of one.

3. My patch includes the reading support for core files:

	* corelow.c (core_xfer_partial): New function.
	(init_core_ops): Use it for core_ops.to_xfer_partial.

   This is something that could not be tested using my patch alone, but
   your "info auxv" command provides the way to test it.  (I did test that
   code, but by using other patches not yet submitted.)

4. You have a single implementation of native_xfer_auxv that presumes that
   /proc/PID/auxv is the thing to use on every potential native system.
   The systems we know of supporting auxv access (Linux and Solaris)
   do all use the same method.  But assuming this does not seem proper or
   consistent with how things are done in gdb.
   
   My patch implements /proc/PID/auxv reading in two places.  This is a bit
   of unsightly duplication, but consistent with the existing duplication
   of code between procfs.c and linux-proc.c.  For Linux, the
   implementation is via defining a macro in nm-linux.h that inftarg.c's
   generic xfer_partial function will use; that is consistent with the
   existing NATIVE_XFER_UNWIND_TABLE macro, and the status quo wherein
   Linux support does not have its own struct target_ops.  The procfs.c
   addition seems like the cleaner approach, defining to_xfer_partial for
   that target.  (That way it also works when using "target procfs", which
   seems proper.)  For Solaris that required the sol-thread.c addition so
   that it would pass down the call to procfs_ops in the same way it does
   for to_xfer_memory (i.e. twiddling inferior_ptid).

5. Your implementation uses pread unconditionally.  Mine doesn't bother to
   use pread even when it's available, and just uses lseek + read.  Full
   portability, including things like older glibcs running on newer
   kernels, requires using configure tests for pread.  I did not bother
   with that because in the real-world case, there will just be one read
   from offset 0 and so lseek won't even be called, just open+read+close.

6. My implementation does not try to support writing.  It would be trivial
   to do so if you think it's worthwhile.  I did not bother because no
   system actually supports changing this data, and I can't see a situation
   in which gdb might ever want to even if it were possible.


My inclination is to add the "info auxv" implementation to my patch,
and leave the other details as I did them.  Comments?


Thanks,
Roland