From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29320 invoked by alias); 5 Dec 2002 21:59: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 29313 invoked from network); 5 Dec 2002 21:59:56 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by sources.redhat.com with SMTP; 5 Dec 2002 21:59:56 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id gB5Lxou07617; Thu, 5 Dec 2002 13:59:50 -0800 X-Authentication-Warning: jackfruit.Stanford.EDU: carlton set sender to carlton@math.stanford.edu using -f To: Elena Zannoni Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] linespec.c, part 5 References: <15829.39563.373999.459749@localhost.redhat.com> <15855.35055.794448.770527@localhost.redhat.com> From: David Carlton Date: Thu, 05 Dec 2002 14:03:00 -0000 In-Reply-To: <15855.35055.794448.770527@localhost.redhat.com> Message-ID: User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-12/txt/msg00189.txt.bz2 On Thu, 5 Dec 2002 12:12:15 -0500, Elena Zannoni said: > David Carlton writes: >> Maybe I'll replace the these comments right after the end of >> decode_line_1: >> >> /* Now, still more helper functions. */ >> >> /* NOTE: carlton/2002-11-07: Some of these have non-obvious side >> effects. In particular, if a function is passed ARGPTR as an >> argument, it modifies what ARGPTR points to. (Typically, it >> advances *ARGPTR past whatever substring it has just looked >> at.) */ >> >> with a comment saying: >> >> /* Now, more helper functions for decode_line_1. Some conventions >> that these functions follow: >> >> Decode_line_1 typically passes along some of its arguments or local >> variables to the subfunctions. It passes the variables by >> reference if they are modified by the subfunction, and by value >> otherwise. >> >> Some of the functions have side effects that don't arise from >> variables that are passed by reference. In particular, if a >> function is passed ARGPTR as an argument, it modifies what ARGPTR >> points to; typically, it advances *ARGPTR past whatever substring >> it has just looked at. (If it doesn't modify *ARGPTR, then the >> function gets passed *ARGPTR instead, which is then called ARG: see >> set_flags, for example.) Also, functions that return a struct >> symtabs_and_lines may modify CANONICAL, as in the description of >> decode_line_1. >> >> If a function returns a struct symtabs_and_lines, then that struct >> will immediately make its way up the call chain to be returned by >> decode_line_1. In particular, all of the functions decode_XXX >> calculate the appropriate struct symtabs_and_lines, under the >> assumption that their argument is of the form XXX. */ >> >> Is that clearer? > better, yes. > BTW, I did a diff -w of decode_compound and the code you removed, and > I noticed this (ignore the line numbers): > @@ -211,7 +231,6 @@ > *argptr = (*p == '\'') ? p + 1 : p; > /* Look up entire name */ > sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab); > - s = (struct symtab *) 0; > if (sym) > return symbol_found (funfirstline, canonical, copy, sym, > NULL, sym_symtab); Oh, sorry, I should have explained that. As you noticed, the code once looked something like s = (struct symtab *) 0; if (sym) goto symbol_found; (there was probably other stuff there). Then my first patch replaced all of the "goto symbol_found"'s by "return symbol_found (a bunch of arguments, including s)". So that would have turned the code into s = (struct symtab *) 0; if (sym) return symbol_found (funfirstline, canonical, copy, sym, s, sym_symtab); except that, since we know that s is 0 there, I replaced the s by NULL, giving us s = (struct symtab *) 0; if (sym) return symbol_found (funfirstline, canonical, copy, sym, NULL, sym_symtab); But at the time of my first patch, I couldn't say for sure if another flow of control might be affected by the "s = (struct symtab *) 0;" line, so I left it in there. This patch, however, allows us to answer that question: the only place in the new decode_compound function where 's' is referred to is in that line that is deleted, and since decode_line_1 immediately returns the value of decode_compound, setting 's' doesn't have any effect. So I deleted it. > OK check it in. Thanks, will do (with the expanded comment mentioned above). David Carlton carlton@math.stanford.edu