This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] An implementation of pipe to send gdb command output to the shell
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Abhijit Halder <abhijit dot k dot halder at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 25 Jul 2011 14:52:12 -0300
- Subject: Re: [PATCH] An implementation of pipe to send gdb command output to the shell
- References: <CAOhZP9x4ftm9uPS2vRGuktkoyDrkJb7yj_PPrcnLagny6_sUJw@mail.gmail.com>
Abhijit Halder <abhijit.k.halder@gmail.com> writes:
> I have implemented a feature which will allow one to pass the output
> of any gdb command to the shell for further processing. The syntax
> will be as follows:
> (gdb) gdb command | `{shell command}`
Thanks for working on this. I have made comments below (nit-picking).
> @@ -358,6 +359,71 @@ prepare_execute_command (void)
> return cleanup;
> }
>
> +char *
> +parse_for_shell_command (char *p)
> +{
If this function is not going to be exported through header files, it
should be static. Also, you should add a comement before the function.
> + char *sh_cmd, *cpos, *spos, *epos;
> + int quote_cnt = 0;
> +
> + if ((sh_cmd = strchr (p, '|')) != NULL)
This is more stylish, but we try to avoid excessive parenthesis (at
least I do), and also checks and attributions on the same line.
Something like:
sh_cmd = strchr (p, '|');
if (sh_cmd)
{
...
> + {
> + spos = p;
> + epos = p + strlen (p) - 1;
> +
> + for (;;)
> + {
The braces are misplaced. It should be:
for (;;)
{
...
> + for (cpos = spos; (cpos = memchr (cpos, '"', (sh_cmd-cpos))) != NULL; cpos++)
This line is too long. You could break it on the semi-colons.
> + cpos = spos;
> + while (isspace(*cpos))
> + cpos++;
Use gdb/cli/cli-utils.c:skip_spaces instead.
> + if (*cpos != '`' || *(cpos+1) != '{')
> + return parse_for_shell_command (cpos);
I think you have check if `cpos' and `cpos + 1' are not NULL in this
case, before dereferencing them.
> + if (*cpos != '`' || *(cpos-1) != '}')
> + return parse_for_shell_command (cpos);
Likewise.
> +/* Run execute_command for P and FROM_TTY. Write output in pipe,
> + do not display it to the screen. */
> +
> +void
> +execute_command_to_pipe (char *p, int from_tty, FILE *pipe)
> +{
This function should be marked as static.
> /* Execute the line P as a command, in the current user context.
> Pass FROM_TTY as second argument to the defining function. */
>
> @@ -368,7 +434,16 @@ execute_command (char *p, int from_tty)
> struct cmd_list_element *c;
> enum language flang;
> static int warned = 0;
> - char *line;
> + char *line, *sh_cmd;
> +
> + if ((sh_cmd = parse_for_shell_command (p)) != NULL)
Same comment about not doing attribution and check on the same line.
> diff -rup src//gdb/ui-file.h dst//gdb/ui-file.h
> --- src//gdb/ui-file.h 2011-05-13 22:58:20.000000000 +0530
> +++ dst//gdb/ui-file.h 2011-07-15 13:48:54.603912199 +0530
> @@ -126,6 +126,9 @@ extern struct ui_file *stdio_fileopen (F
> /* Open NAME returning an STDIO based UI_FILE. */
> extern struct ui_file *gdb_fopen (char *name, char *mode);
>
> +/* Modify the file pointer of an STDIO based UI_FILE. */
Two spaces after the dot.
> 2011-07-21 Abhijit Halder <abhijit.k.halder@symantec.com>
I think you have to add two spaces after the date, and two spaces after
your name.
Thanks again for working on this.