From: David Carlton <carlton@kealia.com>
To: "J. Johnston" <jjohnstn@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA]: add reason code and silent flag to decode_line_1
Date: Mon, 08 Dec 2003 22:33:00 -0000 [thread overview]
Message-ID: <yf21xrfj5x9.fsf@hawaii.kealia.com> (raw)
In-Reply-To: <3FD4F5D8.4000207@redhat.com> (J. Johnston's message of "Mon, 08 Dec 2003 17:06:16 -0500")
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 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.
David Carlton
carlton@kealia.com
next prev parent reply other threads:[~2003-12-08 22:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-12-08 22:06 J. Johnston
2003-12-08 22:33 ` David Carlton [this message]
2003-12-08 22:49 ` J. Johnston
2003-12-08 22:54 ` David Carlton
2003-12-09 21:24 ` J. Johnston
2003-12-09 21:28 ` David Carlton
2003-12-09 21:29 ` Daniel Jacobowitz
2003-12-09 22:47 ` Andrew Cagney
2003-12-10 0:00 ` David Carlton
2003-12-10 0:43 ` J. Johnston
2003-12-17 19:48 ` Elena Zannoni
2003-12-17 20:13 ` Daniel Jacobowitz
2003-12-17 21:48 ` J. Johnston
2003-12-17 22:22 ` J. Johnston
2003-12-29 20:20 ` Elena Zannoni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=yf21xrfj5x9.fsf@hawaii.kealia.com \
--to=carlton@kealia.com \
--cc=gdb-patches@sources.redhat.com \
--cc=jjohnstn@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox