Re: [rfa] linespec.c, part 1

Elena Zannoni
2002-11-08T15:08:44Z

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 <carlton@math.stanford.edu>
>
> * 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;
> }