Re: RFC: CLI clenup and edit command

[Andrew Cagney <ac131313@ges.redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 5955 bytes --]

Fernando,

This patch broke nodebug.exp.  The problem is that, even though there is 
no debug info, there are still minimal symbols and they are sufficient 
for doing the symbol table lookup.  With this patch in place, it refuses 
to allow that case.

See attached patch for my tweak (not committed).

Given this problem,  I think it might be safer, drop the 
get_current_or_default...() function and just stick with 
get/set_current_symtab_and_line().  If a function wants to require 
symbol loading then they should do it explicitly.

Andrew


> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.86
> diff -c -p -r1.86 breakpoint.c
> *** breakpoint.c	5 Sep 2002 01:28:14 -0000	1.86
> --- breakpoint.c	13 Sep 2002 19:23:35 -0000
> ***************
> *** 41,46 ****
> --- 41,47 ----
>   #include "annotate.h"
>   #include "symfile.h"
>   #include "objfiles.h"
> + #include "source.h"
>   #include "linespec.h"
>   #include "completer.h"
>   #include "gdb.h"
> *************** parse_breakpoint_sals (char **address,
> *** 4618,4625 ****
>            current_source_symtab (which is decode_line_1's default).  This
>            should produce the results we want almost all of the time while
>            leaving default_breakpoint_* alone.  */
>         if (default_breakpoint_valid
> ! 	  && (!current_source_symtab
>   	      || (strchr ("+-", (*address)[0]) != NULL)))
>   	*sals = decode_line_1 (address, 1, default_breakpoint_symtab,
>   			       default_breakpoint_line, addr_string);
> --- 4619,4630 ----
>            current_source_symtab (which is decode_line_1's default).  This
>            should produce the results we want almost all of the time while
>            leaving default_breakpoint_* alone.  */
> + 	 
> +       struct symtab_and_line cursal = 
> +       			get_current_or_default_source_symtab_and_line ();
> +

Per patch, should use get_current_source_symtab_and_line().
  	 
	


> Index: scm-lang.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/scm-lang.c,v
> retrieving revision 1.9
> diff -c -p -r1.9 scm-lang.c
> *** scm-lang.c	23 Mar 2002 17:38:13 -0000	1.9
> --- scm-lang.c	13 Sep 2002 19:23:36 -0000
> ***************
> *** 29,34 ****
> --- 29,35 ----
>   #include "c-lang.h"
>   #include "scm-lang.h"
>   #include "scm-tags.h"
> + #include "source.h"
>   #include "gdb_string.h"
>   #include "gdbcore.h"
>   
> *************** scm_unpack (struct type *type, char *val
> *** 133,141 ****
>   static int
>   in_eval_c (void)
>   {
> !   if (current_source_symtab && current_source_symtab->filename)
>       {
> !       char *filename = current_source_symtab->filename;
>         int len = strlen (filename);
>         if (len >= 6 && strcmp (filename + len - 6, "eval.c") == 0)
>   	return 1;
> --- 134,145 ----
>   static int
>   in_eval_c (void)
>   {
> !   struct symtab_and_line cursal =
> ! 	  get_current_or_default_source_symtab_and_line ();

I think this is also wrong.


> + /* Return the current source file for listing and next line to list.
> +    NOTE: The returned sal pc and end fields are not valid. */

Even though the sal.pc and sal.end fields are not valid this function 
should not be leaving them undefined.

> + struct symtab_and_line
> + get_current_source_symtab_and_line (void)
> + {
> +   struct symtab_and_line cursal;
> + 
> +   cursal.symtab = current_source_symtab;
> +   cursal.line = current_source_line;
> +   
> +   return cursal;
> + }
> + 
> + /* Return the current source file for listing and next line to list.
> +    If a file is not set, try and get a default.
> +    It may err out if a default cannot be determined.
> +    Depending on where it is called, it can recurse as the process of
> +    determining a new default may call the caler!
> +    Use get_current_source_symtab_and_line instead to get whatever
> +    we have without erroring out or trying to get a default.
> +    NOTE: The returned sal pc and end fields are not valid. */

Same, sal.pc and sal.end shouldn't be left undefined.

> + struct symtab_and_line
> + get_current_or_default_source_symtab_and_line (void)
> + {
> +   struct symtab_and_line cursal;
> + 
> +   if (!have_full_symbols () && !have_partial_symbols ())
> +     error ("No symbol table is loaded.  Use the \"file\" command.");
> + 
> +   /* Pull in a current source symtab if necessary */
> +   if (current_source_symtab == 0)
> +     select_source_symtab (0);
> +   
> +   cursal.symtab = current_source_symtab;
> +   cursal.line = current_source_line;
> +   
> +   return cursal;
> + }
> + 
> + /* Return the current default file for listing and next line to list
> +    (the returned sal pc and end fields are not valid.)
> +    and set the surrent default to whatever is in SAL */
> +    
> + struct symtab_and_line
> + set_current_source_symtab_and_line (struct symtab_and_line *sal)

Suggest either making the parameter constant or (better?) pass by value.

> + {
> +   struct symtab_and_line cursal;
> +   
> +   cursal.symtab = current_source_symtab;
> +   cursal.line = current_source_line;
> + 
> +   current_source_symtab = sal->symtab;
> +   current_source_line = sal->line;
> +   
> +   return cursal;
> + }


> --- 3968,3986 ----
>   decode_line_spec (char *string, int funfirstline)
>   {
>     struct symtabs_and_lines sals;
> +   struct symtab_and_line cursal;
> +   
>     if (string == 0)
>       error ("Empty line specification.");
> +     
> +   /* We use whatever is set as the current source line. We do not try
> +      and get a default  or it will recursively call us! */  
> +   cursal = get_current_source_symtab_and_line ();
> +   
>     sals = decode_line_1 (&string, funfirstline,
> ! 			cursal.symtab, cursal.line,
>   			(char ***) NULL);
> + 
>     if (*string)
>       error ("Junk at end of line specification: %s", string);
>     return sals;




[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 859 bytes --]

2002-09-21  Andrew Cagney  <ac131313@redhat.com>

	* breakpoint.c (parse_breakpoint_sals): Use
	get_current_source_symtab_and_line instead of
	get_current_or_default_source_symtab_and_line.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.89
diff -u -r1.89 breakpoint.c
--- breakpoint.c	20 Sep 2002 14:58:58 -0000	1.89
+++ breakpoint.c	22 Sep 2002 02:12:02 -0000
@@ -4620,8 +4620,7 @@
          should produce the results we want almost all of the time while
          leaving default_breakpoint_* alone.  */
 	 
-      struct symtab_and_line cursal = 
-      			get_current_or_default_source_symtab_and_line ();
+      struct symtab_and_line cursal = get_current_source_symtab_and_line ();
 			
       if (default_breakpoint_valid
 	  && (!cursal.symtab