This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]