Re: [RFA]: add reason code and silent flag to decode_line_1

["J. Johnston" <jjohnstn@redhat.com>]

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.