This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 4/4] Improved linker-debugger interface
Sergio Durigan Junior wrote:
> On Friday, July 13 2012, Gary Benson wrote:
> > diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> > index c111f04..ebb8c3c 100644
> > --- a/gdb/solib-svr4.c
> > +++ b/gdb/solib-svr4.c
[snip]
> > + /* Check that an appropriate number of arguments has been supplied.
> > + We expect:
> > + arg1: Lmid_t lmid (mandatory)
> > + arg2: struct r_debug *r_debug (mandatory)
> > + arg3: struct link_map *new (optional, for incremental updates) */
>
> I guess you could rename the arguments listed here to 'arg0', 'arg1' and
> 'arg2', because `evaluate_probe_argument' takes these numbers as
> arguments. Or you could explicitly say that here. Otherwise it will
> confuse the reader, IMO.
>
> > + probe_argc = get_probe_argument_count (os->objfile, pi->probe);
> > + if (probe_argc == 2)
> > + action = LM_CACHE_RELOAD;
> > + else if (probe_argc < 2)
> > + return LM_CACHE_INVALIDATE;
>
> This is OK...
>
> > + /* We only currently support the global namespace (PR gdb/11839).
> > + If the probe's r_debug doesn't match the global r_debug then
> > + this event refers to some other namespace and must be ignored. */
> > + info = get_svr4_info ();
> > +
> > + /* Always locate the debug struct, in case it has moved. */
> > + info->debug_base = 0;
> > + locate_base (info);
> > +
> > + debug_base = value_as_address (evaluate_probe_argument (os->objfile,
> > + pi->probe, 1));
>
> ...but what would happen if `evaluate_probe_argument' returned NULL?
> It's better to check this, because `value_as_address' calls `value_type'
> which does not check NULL pointers.
>
> Currently, only the SystemTap backend is implemented, and if it returns
> NULL in this case it would be an error, but it's better to guard your
> code IMO.
Thank you for the review. I've updated the comment and added NULL
checks (amongst other things) here:
http://www.cygwin.com/ml/gdb-patches/2012-07/msg00333.html
Cheers,
Gary
--
http://gbenson.net/