Re: Use external editor in 'commands' command

Alfredo Ortega
2009-01-19T11:45:44Z

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

2009/1/17 Eli Zaretskii <eliz@gnu.org>:
>> Date: Fri, 16 Jan 2009 20:08:06 -0200
>> From: Alfredo Ortega <ortegaalfredo@gmail.com>
>> Cc: gdb-patches@sourceware.org
>>
>> gcc will complain about tmpnamp() like this:
>>
>> test.c:(.text+0x13): warning: the use of `tempnam' is dangerous,
>> better use `mkstemp'
>
> You say "tmpnam ()", but the GCC message is for `tempnam'. Which one
> is it? These are different functions.
>

You are right, I screwed with the test, but the error is the same:

/home/alfred/gdb/gdb-6.8/src/gdb/breakpoint.c:609: warning: the use of
`tmpnam' is dangerous, better use `mkstemp'

I'm posting my latest patch, there are multiple changes:

1) Following the suggestion of Tom Tromey, i made "commands" a prefix
command, and now it is "commands edit n". The change is still
backwards compatible. (Didn't made it to auto-complete like other
commands...need to read a little on how to do this)
2) Now there is a "set external-editor" and "show external-editor",
this variable has precedence over the "EDITOR" environment variable,
(If not set, GDB follows the old behavior with "EDITOR" and "/bin/ex")
3) There is a external_editor() utility function in utils.c that one
should call if needing an external text editor.
4) Following the suggestions of Eli, now I use an auxiliary function
of libiberty for temporary file generation, all strings are dynamic
and there is much better error reporting.

This is a much better patch, but also is a much bigger one (I already
sent the FSF form that Tom suggested), so surely there are plenty of
errors. Corrections are welcomed.
Regards,

Alfredo

2009-01-16 Alfredo Ortega <ortegaalfredo@gmail.com>
* breakpoint.c (commands_command,_initialize_breakpoint):
Add the 'edit' keyword to the 'commands' command to allow the
use of an external editor to add or modify commands.
* utils.c,defs.h (external_editor,initialize_utils):
Added an utility function to return the external text editor of the system.
Added "set external-editor" and "show external-editor" commands to
set/show the external editor variable

2009-01-16 Alfredo Ortega <ortegaalfredo@gmail.com>
* gdb.texinfo, refcard.tex (breakpoint commands, set
external-editor, show external-editor): Added
documentation of the edit option, for editing commands with an
external editor. Also, added a brief description of the "set external-editor"
and "show external-editor" commands.

[-- Attachment #2: commands.diff --]
[-- Type: text/x-diff, Size: 9259 bytes --]

diff -upr OLD/src/gdb/breakpoint.c NEW/src/gdb/breakpoint.c
--- OLD/src/gdb/breakpoint.c 2009-01-14 03:10:29.000000000 -0200
+++ NEW/src/gdb/breakpoint.c 2009-01-19 09:36:06.000000000 -0200
@@ -585,13 +585,20 @@ condition_command (char *arg, int from_t
error (_("No breakpoint number %d."), bnum);
}

+#define COMMANDS_EDCOMMAND "edit"
+
static void
commands_command (char *arg, int from_tty)
{
struct breakpoint *b;
char *p;
- int bnum;
+ int bnum, fsize;
struct command_line *l;
+ char *vitmp = NULL;
+ char *cmdline = NULL;
+ FILE *tmpstream = NULL;
+ char *editor;
+ int sysret;

/* If we allowed this, we would have problems with when to
free the storage, if we change the commands currently
@@ -599,30 +606,102 @@ commands_command (char *arg, int from_tt

if (executing_breakpoint_commands)
error (_("Can't use the \"commands\" command among a breakpoint's commands."));
-
p = arg;
- bnum = get_number (&p);
-
+ /* Edit commands with external editor */
+ if (p && (!strncmp (COMMANDS_EDCOMMAND, p, strlen (COMMANDS_EDCOMMAND))))
+ {
+ /* discard the "edit" command */
+ get_number (&p);
+ bnum = get_number (&p);
+ /* Generates the temporary file name */
+ /* vitmp = tempnam(NULL,".gdb"); this is more secure according to man mkstemp, but gcc complains... */
+ p = NULL;
+ if (!(vitmp = make_temp_file (NULL)))
+ {
+ error (_("Can't create temporary file for editing."));
+ return;
+ }
+ if ((editor = external_editor ()) == NULL)
+ {
+ error (_("External editor not found."));
+ return;
+ }
+ ALL_BREAKPOINTS (b) if (b->number == bnum)
+ {
+ if (&b->commands)
+ {
+ /* commands exists, must dump them to the temporal file */
+ tmpstream = fopen (vitmp, "w");
+ l = b->commands;
+ while (l)
+ {
+ fsize = 0;
+ fsize += fwrite (l->line, 1, strlen (l->line), tmpstream);
+ fsize += fwrite ("\n", 1, strlen ("\n"), tmpstream);
+ if (fsize < strlen (l->line) + 1)
+ {
+ error (_("Error writing to temporary file."));
+ fclose (tmpstream);
+ unlink (vitmp);
+ return;
+ };
+ l = l->next;
+ }
+ fclose (tmpstream);
+ }
+ /* Edit the file */
+ cmdline = xmalloc (strlen (editor) + strlen (vitmp) + 50);
+ sprintf (cmdline, "%s \"%s\"", editor, vitmp);
+ sysret = system (cmdline);
+ xfree (cmdline);
+ if (sysret < 0)
+ {
+ error (_("Editor command failed."));
+ return;
+ }
+ }
+ }
+ else
+ bnum = get_number (&p);
if (p && *p)
error (_("Unexpected extra arguments following breakpoint number."));

- ALL_BREAKPOINTS (b)
- if (b->number == bnum)
- {
- char *tmpbuf = xstrprintf ("Type commands for when breakpoint %d is hit, one per line.",
- bnum);
- struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
- l = read_command_lines (tmpbuf, from_tty, 1);
- do_cleanups (cleanups);
- free_command_lines (&b->commands);
- b->commands = l;
- breakpoints_changed ();
- observer_notify_breakpoint_modified (b->number);
- return;
+ ALL_BREAKPOINTS (b) if (b->number == bnum)
+ {
+ if (vitmp)
+ {
+ /* redirect instream */
+ tmpstream = instream;
+ instream = fopen (vitmp, "r");
+ l = read_command_lines (NULL, from_tty, 1);
+ }
+ else
+ {
+ char *tmpbuf =
+ xstrprintf
+ ("Type commands for when breakpoint %d is hit, one per line.",
+ bnum);
+ struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
+ l = read_command_lines (tmpbuf, from_tty, 1);
+ do_cleanups (cleanups);
+ }
+ free_command_lines (&b->commands);
+ b->commands = l;
+ breakpoints_changed ();
+ observer_notify_breakpoint_modified (b->number);
+ if (vitmp)
+ {
+ /* restore instream */
+ instream = tmpstream;
+ /* erase temporal file */
+ unlink (vitmp);
+ }
+ return;
}
error (_("No breakpoint number %d."), bnum);
}

+
/* Like commands_command, but instead of reading the commands from
input stream, takes them from an already parsed command structure.

@@ -8103,6 +8182,10 @@ Usage is `ignore N COUNT'."));
add_com ("commands", class_breakpoint, commands_command, _("\
Set commands to be executed when a breakpoint is hit.\n\
Give breakpoint number as argument after \"commands\".\n\
+Before the command number you can enter the `edit' keyword, and then you can \n\
+use the external editor to add or modify commands.\n\
+Uses the external-editor variable, EDITOR environment variable or /bin/ex,\n\
+in that precedence.\n\
With no argument, the targeted breakpoint is the last one set.\n\
The commands themselves follow starting on the next line.\n\
Type a line containing \"end\" to indicate the end of them.\n\
diff -upr OLD/src/gdb/defs.h NEW/src/gdb/defs.h
--- OLD/src/gdb/defs.h 2009-01-14 03:10:28.000000000 -0200
+++ NEW/src/gdb/defs.h 2009-01-19 08:57:04.000000000 -0200
@@ -330,6 +330,8 @@ extern int subset_compare (char *, char

extern char *safe_strerror (int);

+extern char *external_editor( void );
+
#define ALL_CLEANUPS ((struct cleanup *)0)

extern void do_cleanups (struct cleanup *);
diff -upr OLD/src/gdb/doc/gdb.texinfo NEW/src/gdb/doc/gdb.texinfo
--- OLD/src/gdb/doc/gdb.texinfo 2009-01-14 03:10:27.000000000 -0200
+++ NEW/src/gdb/doc/gdb.texinfo 2009-01-19 09:26:37.000000000 -0200
@@ -3976,6 +3976,8 @@ follow it immediately with @code{end}; t
With no @var{bnum} argument, @code{commands} refers to the last
breakpoint, watchpoint, or catchpoint set (not to the breakpoint most
recently encountered).
+@item commands edit @r{[}@var{bnum}@r{]}
+This spawns an external editor for adding or editing commands. The final @code{end} is not necessary in this case. @xref{Choosing your Editor}.
@end table

Pressing @key{RET} as a means of repeating the last @value{GDBN} command is
@@ -5384,6 +5386,7 @@ prefer to use Emacs facilities to view s
* List:: Printing source lines
* Specify Location:: How to specify code locations
* Edit:: Editing source files
+* Choosing your Editor:: Specifying your text editor
* Search:: Searching source files
* Source Path:: Specifying source directories
* Machine Code:: Source and machine code
@@ -5587,6 +5590,7 @@ Edit the file containing @var{function}

@end table

+@node Choosing your Editor
@subsection Choosing your Editor
You can customize @value{GDBN} to use any editor you want
@footnote{
@@ -5611,6 +5615,19 @@ or in the @code{csh} shell,
setenv EDITOR /usr/bin/vi
gdb @dots{}
@end smallexample
+Another option is to use the @code{set external-editor} command:
+
+@table @code
+@item set external-editor
+@kindex set external-editor
+This command controls the external text editor that internal gdb commands use. The external-editor variable has precedence over the @code{EDITOR} enviroment variable.
+@end table
+
+@table @code
+@item show external-editor
+@kindex show external-editor
+This command shows the external text editor that internal gdb commands use.
+@end table

@node Search
@section Searching Source Files
diff -upr OLD/src/gdb/doc/refcard.tex NEW/src/gdb/doc/refcard.tex
--- OLD/src/gdb/doc/refcard.tex 2009-01-14 03:10:27.000000000 -0200
+++ NEW/src/gdb/doc/refcard.tex 2009-01-19 08:57:19.000000000 -0200
@@ -355,10 +355,9 @@ delete when reached
ignore {\it n} {\it count}&ignore breakpoint {\it n}, {\it count}
times\cr
\cr
-commands {\it n}\par
+commands \opt{{\it edit}} {\it n}\par
\qquad \opt{\tt silent}\par
-\qquad {\it command-list}&execute GDB {\it command-list} every time breakpoint {\it n} is reached. \opt{{\tt silent} suppresses default
-display}\cr
+\qquad {\it command-list}&execute GDB {\it command-list} every time breakpoint {\it n} is reached \opt{{\tt edit} using external editor}. \opt{{\tt silent} suppresses default display} \cr
end&end of {\it command-list}\cr
\endsec

diff -upr OLD/src/gdb/utils.c NEW/src/gdb/utils.c
--- OLD/src/gdb/utils.c 2009-01-14 03:10:27.000000000 -0200
+++ NEW/src/gdb/utils.c 2009-01-19 09:25:19.000000000 -0200
@@ -96,6 +96,10 @@ static void prompt_for_continue (void);
static void set_screen_size (void);
static void set_width (void);

+/* External text editor */
+
+static char *external_editor_command = NULL;
+
/* A flag indicating whether to timestamp debugging messages. */

static int debug_timestamp = 0;
@@ -2694,6 +2698,12 @@ When set, debugging messages will be mar
NULL,
show_debug_timestamp,
&setdebuglist, &showdebuglist);
+ add_setshow_filename_cmd ("external-editor", no_class, &external_editor_command, _("\
+Set the external text editor that gdb uses."),
+ _("\
+Show the external text editor."), NULL,
+ NULL, NULL,
+ &setlist, &showlist);
}

/* Machine specific function to handle SIGWINCH signal. */
@@ -3443,3 +3453,16 @@ gdb_buildargv (const char *s)
nomem (0);
return argv;
}
+
+/* Returns the external editor */
+char *
+external_editor (void)
+{
+ char *editor;
+ if (external_editor_command)
+ return external_editor_command;
+ if ((editor = (char *) getenv ("EDITOR")) == NULL)
+ editor = "/bin/ex";
+ return editor;
+
+}