[RFA]: add reason code and silent flag to decode_line_1
J. Johnston
jjohnstn@redhat.com
Mon Dec 8 22:49:00 GMT 2003
David Carlton wrote:
> On Mon, 08 Dec 2003 17:06:16 -0500, "J. Johnston" <jjohnstn@redhat.com> said:
>
>
>>I have added two new paramters to decode_line_1. One is a
>>reason_code field and the other is a silent_flag. The reason code
>>is a pointer to an int to store a reason code should the function
>>cause an error. At present, the reason_code is only set if the file
>>or function is not found. I would have called it not_found_ptr
>>instead of reason_code_ptr but I felt this could be enhanced in the
>>future to include other forms of error that could be of interest to
>>the caller. The reason code works like errno whereby the caller is
>>expected to clear the field before calling and check it afterwards.
>>The silent_if_not_found flag tells the function not to issue an
>>error message if the function is to fail because the function/source
>>file is not found.
>
>
> I didn't even know GDB had a function 'throw_exception'; learn
> something new every day. Anyways, I'm not in a position to approve
> it, but here are my thoughts. My first reaction was "decode_line_1
> has too many arguments to begin with", but that's not your fault. But
> I'm not sure you have a coherent story for your two arguments - on the
> one hand, you want to be general with reason_code_ptr, but on the
> other hand silent_if_not_found is very specialized, and they both
> convey overlapping information.
>
> It seems reasonable to me that you can say "if we fail for certain
> reasons, then either we'll convey the reason for failure by calling
> error() with an error message or else we'll convey the reason for
> failure by storing it in a variable and throwing an exception". I
> have a hard time imagining situations where silent_if_not_found is
> false but reason_code_ptr is still used, for example. (Assuming, of
> course, that further reasons aren't added; and, if they are added,
> we'd need to add another silent_if_XXX flag.)
>
> So I think that you should just add the reason_code_ptr variable (and
> probably just call it not_found_ptr for now, though I don't feel
> strongly about that), and have the exception be thrown if and only if
> it is non-NULL.
>
I like that solution. I'll resubmit the patch.
> (I also think that, if I ever finish my decode_line_1 cleanup, I
> should think about reducing the number of its arguments, and I think
> that we should switch GDB over to a language with a better exception
> model, but those aren't issues that you need to deal with right
> now. :-) )
>
> A unit test would be nice, too, if possible.
>
Well, that comes with my pending breakpoint support which I was asked to break
into smaller chunks and get the support structure in place first. It will be
the first and perhaps only user of this functionality.
-- Jeff J.
More information about the Gdb-patches
mailing list