Re: [PATCH] Re-posting of proposed patch

[Fernando Nasser <fnasser at redhat dot com>]

Geroge,

A few quick questions and some notes.

No need to update the patch as I will check it in with the necessary
changes as part of a clean-up that I am doing.  But you can use the
info for your next patch. Anyway, I need to know a couple of things too.

Thanks for the cool feature.  I am eager to start using it.

Best regards,
Fernando


George Helffrich +44 117 954 5437 wrote:
> All -
>
>      I posted this patch 2 July, and while the doc portions were approved
> (Eli Zaretskii), the remainder doesn't appear to have been looked at.  In
> the hope that it was simply overlooked in the beehive of other work evident
> from the list's activity, I'm re-posting it to facilitate approval by those
> responsible.
>
> 					 George Helffrich
>
>
> 2002-07-02  George Helffrich <george@gly.bris.ac.uk>
>
>       * cli/cli-cmds.c, defs.h, source.c, gdb.1, doc/gdb.texinfo,
>       doc/refcard.tex: Add edit command.  This command mimics the behavior
>       of Sun and SGI dbx, and invokes an editor at the current line number.
>       Editor comes from EDITOR environment variable, or defaults to /bin/ex.
Each file goes in a separate '*' line.  It is valid to be more 
descriptive in the file with the main changes (source.c) in your case, 
and refer to it in the follwing entries.  Look at other ChangeLog 
entries for examples.



> Index: gdb-5.1.1/src/gdb/defs.h
> ===================================================================
> RCS file: /usr/src/local/cvsroot/gdb-5.1.1/src/gdb/defs.h,v
> retrieving revision 1.1.1.1
> retrieving revision 1.2
> diff -c -r1.1.1.1 -r1.2
> *** gdb-5.1.1/src/gdb/defs.h	2002/02/02 11:34:18	1.1.1.1
> --- gdb-5.1.1/src/gdb/defs.h	2002/05/26 12:12:26	1.2
> ***************
> *** 751,756 ****
> --- 751,760 ----
>   
>   extern void print_address (CORE_ADDR, struct ui_file *);
>   
> + /* From command.c */
> + 
> + extern void shell_escape (char *, int);
> + 
>   /* From source.c */
>   
>   extern int openp (const char *, int, const char *, int, int, char **);
I am removing this part, as we can't export anything from the cli.  The 
cli uses libgdb, not vice-versa.  Each UI is a shell on the top of libgdb.


> Index: gdb-5.1.1/src/gdb/gdb.1
> ===================================================================
> RCS file: /usr/src/local/cvsroot/gdb-5.1.1/src/gdb/gdb.1,v
> retrieving revision 1.2
> retrieving revision 1.3
(snip...)
> Index: gdb-5.1.1/src/gdb/doc/gdb.texinfo
> ===================================================================
> RCS file: /usr/src/local/cvsroot/gdb-5.1.1/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.1
> retrieving revision 1.2.2.1
> diff -c -r1.1 -r1.2.2.1
(snip...)

I had problems cecking those in.  Can you repost an updated version of 
the documentation part only that applies to more current sources?  Thanks.



> Index: gdb-5.1.1/src/gdb/source.c
> ===================================================================
> RCS file: /usr/src/local/cvsroot/gdb-5.1.1/src/gdb/source.c,v
> retrieving revision 1.2
> retrieving revision 1.3.2.2
> diff -c -r1.2 -r1.3.2.2
> *** gdb-5.1.1/src/gdb/source.c	2002/05/25 20:27:13	1.2
> --- gdb-5.1.1/src/gdb/source.c	2002/07/02 10:32:21	1.3.2.2
> ***************
> *** 82,87 ****
> --- 82,91 ----
>   
>   static void list_command (char *, int);
>   
> + static void edit_command (char *, int);
> + 
> + static void do_edit (struct symtab *, int, int);
> + 
>   static void ambiguous_line_spec (struct symtabs_and_lines *);
>   
>   static void source_info (char *, int);
> ***************
> *** 1443,1448 ****
> --- 1447,1563 ----
>     xfree (sals.sals);
>   }
>   
> + static void
> + edit_command (char *arg, int from_tty)
> + {
> +   struct symtabs_and_lines sals, sals_end;
> +   struct symtab_and_line sal, sal_end;
sals_end and sal_end are not used.  You can detect this things with the 
proper compiler option.


> +   struct symbol *sym;
> +   char *arg1;
> + 
> +   if (!have_full_symbols () && !have_partial_symbols())
> +     error ("No symbol table is loaded.  Use the \"file\" command.");
Just "No symbol table is loaded." is fine.  We sometimes say "Use 
<command>" but it is in a different context (the command would fix the 
condition).

> + 
> +   /* Pull in a current source symtab if necessary */
> +   if (current_source_symtab == 0 &&
> +       (arg == 0 || arg[0] == '+' || arg[0] == '-'))
+ and - are not part of your syntax, right?  In any case you don't 
handle them after, so you would either have to add code for those or 
remove these options.  IMO + and - would not make much sense for edit, 
but I am asking just in case (This was my main question)


> +     select_source_symtab (0);
> + 
> +   /* bare "edit" edits file with present line.  */
> + 
> +   if (arg == 0)
> +     {
> +       if (current_source_symtab == 0)
> + 	error ("No default source file yet.  Do \"help edit\".");
> +       do_edit (current_source_symtab,
> +                current_source_line + lines_to_list/2, from_tty);
> +       return;
> +     }
> + 
> +   /* Now should only be one argument -- decode it in SAL */
> + 
> +   arg1 = arg;
> +   sals = decode_line_1 (&arg1, 0, 0, 0, 0);
> + 
> +   if (! sals.nelts) return;  /*  C++  */
> +   if (sals.nelts > 1) {
> +     ambiguous_line_spec (&sals);
> +     free (sals.sals);

We use xfree(), instead of free()

> +     return;
> +   }
> + 
> +   sal = sals.sals[0];
> +   free (sals.sals);
Likewise.

> + 
> +   if (*arg1)
> +     error ("Junk at end of line specification.");
> + 
> +   /* if line was specified by address,
> +      first print exactly which line, and which file.
> +      In this case, sal.symtab == 0 means address is outside
> +      of all known source files, not that user failed to give a filename.  */
> +   if (*arg == '*')
> +     {
> +       if (sal.symtab == 0)
> + 	/* FIXME-32x64--assumes sal.pc fits in long.  */
> + 	error ("No source file for address %s.",
> + 		local_hex_string((unsigned long) sal.pc));
> +       sym = find_pc_function (sal.pc);
> +       if (sym)
> + 	{
> + 	  print_address_numeric (sal.pc, 1, gdb_stdout);
> + 	  printf_filtered (" is in ");
> + 	  fputs_filtered (SYMBOL_SOURCE_NAME (sym), gdb_stdout);
> + 	  printf_filtered (" (%s:%d).\n", sal.symtab->filename, sal.line);
> + 	}
> +       else
> + 	{
> + 	  print_address_numeric (sal.pc, 1, gdb_stdout);
> + 	  printf_filtered (" is at %s:%d.\n",
> + 			   sal.symtab->filename, sal.line);
> + 	}
> +     }
> + 
> +   /* If what was given does not imply a symtab, it must be an undebuggable
> +      symbol which means no source code.  */
> + 
> +   if (sal.symtab == 0)
> +     error ("No line number known for %s.", arg);
> + 
> +   do_edit(sal.symtab, sal.line, from_tty);
> + }
> + 
> + static void
> + do_edit (struct symtab *symtb, int line, int from_tty)
> + {
> +   int cmdlen, log10;
> +   unsigned m;
> +   char *editor;
> +   char *p;
> + 
> +   if ((editor = (char *) getenv ("EDITOR")) == NULL)
> +       editor = "/bin/ex";
> +   
> +   /* Approximate base-10 log of line to 1 unit for digit count */
> +   for(log10=32, m=0x80000000; !(line & m) && log10>0; log10--, m=m>>1);
> +   log10 = 1 + (int)((log10 + (0 == ((m-1)&line)))/3.32192809);
> + 
> +   cmdlen = strlen(editor) + 1
> +          + (NULL == symtb->dirname ? 0 : strlen(symtb->dirname) + 1)
> + 	 + (NULL == symtb->filename? 0 : strlen(symtb->filename)+ 1)
> + 	 + log10 + 2;
> +   
> +   p = xmalloc(cmdlen);
> +   sprintf(p,"%s +%d %s%s",editor,line,
> +      (NULL == symtb->dirname ? "./" :
> +         (NULL != symtb->filename && *(symtb->filename) != '/') ?
> + 	   symtb->dirname : ""),
> +      (NULL == symtb->filename ? "unknown" : symtb->filename)
> +   );
> +   shell_escape(p, from_tty);
> +   free(p);
> + }
> + 
>   /* Commands to search the source file for a regexp.  */
>   
>   /* ARGSUSED */
> ***************
> *** 1727,1732 ****
> --- 1842,1860 ----
>   
>     if (dbx_commands)
>       add_com_alias ("file", "list", class_files, 1);
> + 
> +   c = add_com ("edit", class_files, edit_command,
> +            concat ("Edit specified file or function.\n\
> + With no argument, edits file containing most recent line listed.\n\
> + ", "\
> + Editing targets can be specified in these ways:\n\
> +   FILE:LINENUM, to edit at that line in that file,\n\
> +   FUNCTION, to edit at the beginning of that function,\n\
> +   FILE:FUNCTION, to distinguish among like-named static functions.\n\
> +   *ADDRESS, to edit at the line containing that address.\n\
> + Uses EDITOR environment variable contents as editor (or ex as default).",NULL));
> + 
> +   c->completer = location_completer;
>   
>     add_show_from_set
>       (add_set_cmd ("listsize", class_support, var_uinteger,

--
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9