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 Wednesday, July 18 2012, Jan Kratochvil wrote:
> 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.
>> > + 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.
I will not argue.
--
Sergio