[PATCH 2/2] Catching errors on probes-based dynamic linker interface
Sergio Durigan Junior
sergiodj@redhat.com
Tue Sep 1 16:26:00 GMT 2015
On Tuesday, September 01 2015, Gary Benson wrote:
>> > On Tuesday, August 25 2015, Gary Benson wrote:
>> > > Sergio Durigan Junior wrote:
>> > > > On Monday, August 24 2015, Gary Benson wrote:
>> > > > > Maybe this would be clearer and more robust:
>> > > > >
>> > > > > TRY
>> > > > > {
>> > > > > unsigned probe_argc;
>> > > > >
>> > > > > probe_argc = get_probe_argument_count (pa->probe, frame);
>> > > > >
>> > > > > if (probe_argc == 2)
>> > > > > action = FULL_RELOAD;
>> > > > > else if (probe_argc < 2)
>> > > > > action = PROBES_INTERFACE_FAILED;
>> > > > > }
>> > > > > CATCH (ex, RETURN_MASK_ERROR)
>> > > > > {
>> > > > > exception_print (gdb_stderr, ex);
>> > > > > action = PROBES_INTERFACE_FAILED;
>> > > > > }
>> > > > > END_CATCH
>> > > >
>> > > > Maybe it's a matter of preference, but I don't like this (and
>> > > > I don't see why it is more robust). I prefer to have as
>> > > > little code as possible running on the TRY block, and handle
>> > > > everything else outside of it. I think it also makes things a
>> > > > bit more confuse because you have two places where action can
>> > > > be PROBES_INTERFACE_FAILED.
>> > >
>> > > Well, there are two different failures:
>> > >
>> > > 1) get_probe_argument_count failed
>> > > 2) get_probe_argument_count returned < 2
>> >
>> > Yes, and both are covered by the proposed patch. It is not really
>> > important to distinguish between these failures today: what really
>> > matters is that GDB recognizes both as failures and acts
>> > accordingly.
>
> That matters. It also matters that future maintainers do not trip
> over this.
>
> I am ok with doing this:
>
> TRY
> {
> probe_argc = get_probe_argument_count (pa->probe, frame);
> }
> CATCH (ex, RETURN_MASK_ERROR)
> {
> exception_print (gdb_stderr, ex);
> probe_argc = 0;
> }
> END_CATCH
>
> If you put a big fat comment above the following block, e.g.:
>
> /* Note that failure of get_probe_argument_count will
> set probe_argc == 0. This must result in returning
> action = PROBES_INTERFACE_FAILED. */
> if (probe_argc == 2)
> action = FULL_RELOAD;
> else if (probe_argc < 2)
> action = PROBES_INTERFACE_FAILED;
Great, that works for me as well. I will update the patch here to
address this.
Thanks, Gary.
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
More information about the Gdb-patches
mailing list