From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8861 invoked by alias); 8 Nov 2002 15:08:44 -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 8790 invoked from network); 8 Nov 2002 15:08:42 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 8 Nov 2002 15:08:42 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id gA8Ejtw28111 for ; Fri, 8 Nov 2002 09:45:55 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id gA8F8ff16740; Fri, 8 Nov 2002 10:08:42 -0500 Received: from localhost.redhat.com (romulus-int.sfbay.redhat.com [172.16.27.46]) by pobox.corp.redhat.com (8.11.6/8.11.6) with ESMTP id gA8F8ds07282; Fri, 8 Nov 2002 10:08:39 -0500 Received: by localhost.redhat.com (Postfix, from userid 469) id 7E106FF79; Fri, 8 Nov 2002 10:04:32 -0500 (EST) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15819.53888.241829.671843@localhost.redhat.com> Date: Fri, 08 Nov 2002 07:08:00 -0000 To: David Carlton Cc: gdb-patches@sources.redhat.com, Elena Zannoni , Fernando Nasser , Jim Blandy Subject: Re: [rfa] linespec.c, part 1 In-Reply-To: References: X-SW-Source: 2002-11/txt/msg00221.txt.bz2 Looks good. testsuite results are same, compiled with ,-Werror, right ? Check it in. Thanks Elena David Carlton writes: > This patch gets rid of the goto's and move some code into separate > functions 'symbol_found' and 'minsym_found'. It's based on the > following observations (I'll do them for symbol_found, but it's the > same for both): > > 1) The code after the label symbol_found: looks like this > > symbol_found: > if (sym != NULL) > { > /* initialize values appropriately, and then either return > values or signal an error. > } > > So we can move the code inside the braces into a separate function > symbol_found that returns a struct symtab_and_lines, turning this > into: > > symbol_found: > if (sym != NULL) > return symbol_found (args); > > 2) In all situations where there is a 'goto symbol_found', it is > immediately preceded by a check that sym is non-NULL. So the > behavior of the program isn't changed by if we leave the gotos > intact but further rewrite the above code as > > if (sym != NULL) > symbol_found: > return symbol_found (args); > > 3) So, given that this is the case, we might as well replace each > 'goto symbol_found' by 'return symbol_found (args);'. > > The patch gets a little weird to read towards the end: it would seem > that the algorithms that patch -u uses don't always work the best if > you're moving chunks of code around. Sorry about that. > > David Carlton > carlton@math.stanford.edu > > 2002-11-07 David Carlton > > * linespec.c (symbol_found): New function. > (minsym_found): New function. > (decode_line_1): Separate out some code into separate functions. > > Index: linespec.c > =================================================================== > RCS file: /cvs/src/src/gdb/linespec.c,v > retrieving revision 1.25 > diff -u -p -r1.25 linespec.c > --- linespec.c 24 Oct 2002 21:02:53 -0000 1.25 > +++ linespec.c 7 Nov 2002 21:08:18 -0000 > @@ -53,6 +53,18 @@ static char *find_toplevel_char (char *s > static struct symtabs_and_lines decode_line_2 (struct symbol *[], > int, int, char ***); > > +static struct > +symtabs_and_lines symbol_found (int funfirstline, > + char ***canonical, > + char *copy, > + struct symbol *sym, > + struct symtab *s, > + struct symtab *sym_symtab); > + > +static struct > +symtabs_and_lines minsym_found (int funfirstline, > + struct minimal_symbol *msymbol); > + > /* Helper functions. */ > > /* Issue a helpful hint on using the command completion feature on > @@ -910,16 +922,9 @@ decode_line_1 (char **argptr, int funfir > /* Look up entire name */ > sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab); > s = (struct symtab *) 0; > - /* Prepare to jump: restore the " if (condition)" so outer layers see it */ > - /* Symbol was found --> jump to normal symbol processing. > - Code following "symbol_found" expects "copy" to have the > - symbol name, "sym" to have the symbol pointer, "s" to be > - a specified file's symtab, and sym_symtab to be the symbol's > - symtab. */ > - /* By jumping there we avoid falling through the FILE:LINE and > - FILE:FUNC processing stuff below */ > if (sym) > - goto symbol_found; > + return symbol_found (funfirstline, canonical, copy, sym, > + NULL, sym_symtab); > > /* Couldn't find any interpretation as classes/namespaces, so give up */ > /* The quotes are important if copy is empty. */ > @@ -985,12 +990,9 @@ decode_line_1 (char **argptr, int funfir > sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab); > if (sym) > { > - /* Yes, we have a symbol; jump to symbol processing */ > - /* Code after symbol_found expects S, SYM_SYMTAB, SYM, > - and COPY to be set correctly */ > *argptr = (*p == '\'') ? p + 1 : p; > - s = (struct symtab *) 0; > - goto symbol_found; > + return symbol_found (funfirstline, canonical, copy, sym, > + NULL, sym_symtab); > } > /* Otherwise fall out from here and go to file/line spec > processing, etc. */ > @@ -1151,19 +1153,16 @@ decode_line_1 (char **argptr, int funfir > sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab); > s = (struct symtab *) 0; > need_canonical = 1; > - /* Symbol was found --> jump to normal symbol processing. > - Code following "symbol_found" expects "copy" to have the > - symbol name, "sym" to have the symbol pointer, "s" to be > - a specified file's symtab, and sym_symtab to be the symbol's > - symtab. */ > + /* Symbol was found --> jump to normal symbol processing. */ > if (sym) > - goto symbol_found; > + return symbol_found (funfirstline, canonical, copy, sym, > + NULL, sym_symtab); > > /* If symbol was not found, look in minimal symbol tables */ > msymbol = lookup_minimal_symbol (copy, NULL, NULL); > /* Min symbol was found --> jump to minsym processing. */ > if (msymbol) > - goto minimal_symbol_found; > + return minsym_found (funfirstline, msymbol); > > /* Not a user variable or function -- must be convenience variable */ > need_canonical = (s == 0) ? 1 : 0; > @@ -1196,84 +1195,92 @@ decode_line_1 (char **argptr, int funfir > : get_selected_block (0)), > VAR_NAMESPACE, 0, &sym_symtab); > > -symbol_found: /* We also jump here from inside the C++ class/namespace > - code on finding a symbol of the form "A::B::C" */ > - > if (sym != NULL) > - { > - if (SYMBOL_CLASS (sym) == LOC_BLOCK) > - { > - /* Arg is the name of a function */ > - values.sals = (struct symtab_and_line *) > - xmalloc (sizeof (struct symtab_and_line)); > - values.sals[0] = find_function_start_sal (sym, funfirstline); > - values.nelts = 1; > + return symbol_found (funfirstline, canonical, copy, sym, s, sym_symtab); > > - /* Don't use the SYMBOL_LINE; if used at all it points to > - the line containing the parameters or thereabouts, not > - the first line of code. */ > + msymbol = lookup_minimal_symbol (copy, NULL, NULL); > > - /* We might need a canonical line spec if it is a static > - function. */ > - if (s == 0) > - { > - struct blockvector *bv = BLOCKVECTOR (sym_symtab); > - struct block *b = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK); > - if (lookup_block_symbol (b, copy, NULL, VAR_NAMESPACE) != NULL) > - build_canonical_line_spec (values.sals, copy, canonical); > - } > - return values; > - } > - else > - { > - if (funfirstline) > - error ("\"%s\" is not a function", copy); > - else if (SYMBOL_LINE (sym) != 0) > - { > - /* We know its line number. */ > - values.sals = (struct symtab_and_line *) > - xmalloc (sizeof (struct symtab_and_line)); > - values.nelts = 1; > - memset (&values.sals[0], 0, sizeof (values.sals[0])); > - values.sals[0].symtab = sym_symtab; > - values.sals[0].line = SYMBOL_LINE (sym); > - return values; > - } > - else > - /* This can happen if it is compiled with a compiler which doesn't > - put out line numbers for variables. */ > - /* FIXME: Shouldn't we just set .line and .symtab to zero > - and return? For example, "info line foo" could print > - the address. */ > - error ("Line number not known for symbol \"%s\"", copy); > - } > - } > + if (msymbol != NULL) > + return minsym_found (funfirstline, msymbol); > > - msymbol = lookup_minimal_symbol (copy, NULL, NULL); > + if (!have_full_symbols () && > + !have_partial_symbols () && !have_minimal_symbols ()) > + error ("No symbol table is loaded. Use the \"file\" command."); > > -minimal_symbol_found: /* We also jump here from the case for variables > - that begin with '$' */ > + error ("Function \"%s\" not defined.", copy); > + return values; /* for lint */ > +} > > - if (msymbol != NULL) > +static struct symtabs_and_lines > +symbol_found (int funfirstline, char ***canonical, char *copy, > + struct symbol *sym, struct symtab *s, > + struct symtab *sym_symtab) > +{ > + struct symtabs_and_lines values; > + > + if (SYMBOL_CLASS (sym) == LOC_BLOCK) > { > + /* Arg is the name of a function */ > values.sals = (struct symtab_and_line *) > xmalloc (sizeof (struct symtab_and_line)); > - values.sals[0] = find_pc_sect_line (SYMBOL_VALUE_ADDRESS (msymbol), > - (struct sec *) 0, 0); > - values.sals[0].section = SYMBOL_BFD_SECTION (msymbol); > - if (funfirstline) > + values.sals[0] = find_function_start_sal (sym, funfirstline); > + values.nelts = 1; > + > + /* Don't use the SYMBOL_LINE; if used at all it points to > + the line containing the parameters or thereabouts, not > + the first line of code. */ > + > + /* We might need a canonical line spec if it is a static > + function. */ > + if (s == 0) > { > - values.sals[0].pc += FUNCTION_START_OFFSET; > - values.sals[0].pc = SKIP_PROLOGUE (values.sals[0].pc); > + struct blockvector *bv = BLOCKVECTOR (sym_symtab); > + struct block *b = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK); > + if (lookup_block_symbol (b, copy, NULL, VAR_NAMESPACE) != NULL) > + build_canonical_line_spec (values.sals, copy, canonical); > } > - values.nelts = 1; > return values; > } > + else > + { > + if (funfirstline) > + error ("\"%s\" is not a function", copy); > + else if (SYMBOL_LINE (sym) != 0) > + { > + /* We know its line number. */ > + values.sals = (struct symtab_and_line *) > + xmalloc (sizeof (struct symtab_and_line)); > + values.nelts = 1; > + memset (&values.sals[0], 0, sizeof (values.sals[0])); > + values.sals[0].symtab = sym_symtab; > + values.sals[0].line = SYMBOL_LINE (sym); > + return values; > + } > + else > + /* This can happen if it is compiled with a compiler which doesn't > + put out line numbers for variables. */ > + /* FIXME: Shouldn't we just set .line and .symtab to zero > + and return? For example, "info line foo" could print > + the address. */ > + error ("Line number not known for symbol \"%s\"", copy); > + } > +} > > - if (!have_full_symbols () && > - !have_partial_symbols () && !have_minimal_symbols ()) > - error ("No symbol table is loaded. Use the \"file\" command."); > +static struct symtabs_and_lines > +minsym_found (int funfirstline, struct minimal_symbol *msymbol) > +{ > + struct symtabs_and_lines values; > > - error ("Function \"%s\" not defined.", copy); > - return values; /* for lint */ > + values.sals = (struct symtab_and_line *) > + xmalloc (sizeof (struct symtab_and_line)); > + values.sals[0] = find_pc_sect_line (SYMBOL_VALUE_ADDRESS (msymbol), > + (struct sec *) 0, 0); > + values.sals[0].section = SYMBOL_BFD_SECTION (msymbol); > + if (funfirstline) > + { > + values.sals[0].pc += FUNCTION_START_OFFSET; > + values.sals[0].pc = SKIP_PROLOGUE (values.sals[0].pc); > + } > + values.nelts = 1; > + return values; > }