From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20511 invoked by alias); 8 Dec 2003 22:33:57 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 20374 invoked from network); 8 Dec 2003 22:33:39 -0000 Received: from unknown (HELO hawaii.kealia.com) (209.3.10.89) by sources.redhat.com with SMTP; 8 Dec 2003 22:33:39 -0000 Received: by hawaii.kealia.com (Postfix, from userid 2049) id 35A2EC6BB; Mon, 8 Dec 2003 14:33:38 -0800 (PST) To: "J. Johnston" Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA]: add reason code and silent flag to decode_line_1 References: <3FD4F5D8.4000207@redhat.com> From: David Carlton Date: Mon, 08 Dec 2003 22:33:00 -0000 In-Reply-To: <3FD4F5D8.4000207@redhat.com> (J. Johnston's message of "Mon, 08 Dec 2003 17:06:16 -0500") Message-ID: User-Agent: Gnus/5.1002 (Gnus v5.10.2) XEmacs/21.4 (Rational FORTRAN, linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-12/txt/msg00263.txt.bz2 On Mon, 08 Dec 2003 17:06:16 -0500, "J. Johnston" 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