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
On Tue, 17 Jul 2012 20:11:07 +0200, Sergio Durigan Junior wrote:
> On Friday, July 13 2012, Gary Benson wrote:
> > + 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.
Could you clarify the 1-counting vs. 0-count in: evaluate_probe_argument,
compile_to_ax. Maybe it is not fully clear even in
sym_evaluate_probe_argument, sym_compile_to_ax.
(It is already clear in probe_safe_evaluate_at_pc.)
OK to check it in as obvious if the new text is clear enough (0 <= x < count).
> > + 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.
Currently the API comment defines "returning a value corresponding to it.".
There is no "or NULL if evaluation error occurs" or anything like that,
therefore it IMNSHO means the returned value is non-NULL.
Therefore I find correct for Gary to assume the returned value is non-NULL.
Thanks,
Jan