This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] An implementation of pipe to make I/O communication between gdb and shell.
Hi Abhijit,
Some minor nits (again).
Abhijit Halder <abhijit.k.halder@gmail.com> writes:
> +struct pipe_obj
> +{
> + /* The delimiter to separate out gdb-command and shell-command. This can be
^^
Two spaces after the period.
> + /* The pex object use to create pipeline between gdb and shell. */
Some typos:
"/*The pex object used to create a pipeline between GDB and shell. */"
> + if (*p != '\0') *p++ = '\0';
I'm not found of this style. Please, put the assignment on the next
line:
if (*p != '\0')
*p++ = '\0';
> + for (pipe->shell_cmd = "";
Sorry, I didn't understand this line. Is this needed? If so, I think
it's better if you do `pipe->shell_cmd = NULL'.
> + int mismatch = memcmp (p, pipe->dlim, (separator-p));
Space between `separator', `-' and `p'. If you want, you can put this
`memcmp' call directly on the `if' condition, and then you wouldn't need
the braces on the `for'.
> +/* Run execute_command for P and FROM_TTY. Write output to the pipe, do not
^^
Two spaces after period.
> + if (pipe->handle == NULL)
> + error (_("Failed to create pipe"));
> +
> + {
> + int status;
> + const char *err
> + = pex_run (pipe->pex,
> + PEX_SEARCH | PEX_LAST | PEX_STDERR_TO_STDOUT,
> + argv[0], argv,
> + NULL, NULL,
> + &status);
> + if (err)
> + error (_("Failed to execute %s"), argv[0]);
> +
> + do_cleanups (cleanup);
> + }
Why the braces?
> + if (pipe->pex)
> + {
> + int status;
> +
> + pex_get_status (pipe->pex, 1, &status);
> + pex_free (pipe->pex);
Do you really need to call `pex_get_status' here? I'm really asking,
because I don't know about pex.
Thanks for your work on this patch.
Regards,
Sergio.