Re: [PATCH] An implementation of pipe to make I/O communication between gdb and shell. [MinGW question]

[Abhijit Halder <abhijit dot k dot halder at gmail dot com>]

On Sun, Aug 21, 2011 at 12:45 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 16 Aug 2011 14:45:19 +0200, Abhijit Halder wrote:
>> +#include "defs.h"
>> +#include <ctype.h>
>> +#include "gdb_string.h"
>> +#include "ui-file.h"
>> +#include "ui-out.h"
>> +#include "cli/cli-utils.h"
>> +#include "gdbcmd.h"
>> +#include "libiberty.h"
>> +#include "exceptions.h"
>> +
>> +#if defined(__MINGW32__)
>> +#define SHELL "cmd.exe"
>> +#define OPTION_TO_SHELL "/c"
>> +#else
>> +#define SHELL "/bin/sh"
>> +#define OPTION_TO_SHELL "-c"
>> +#endif
>
> GNU Coding style would say:
> #if defined (__MINGW32__)
> but #ifdef would be also OK IMO.
>
> and indent, please:
>
> # define SHELL "cmd.exe"
> # define OPTION_TO_SHELL "/c"
> #else
> # define SHELL "/bin/sh"
> # define OPTION_TO_SHELL "-c"
> #endif
>
> It would be nice to get the __MINGW32__ tested but OK to check it in if no
> test appears, some __MINGW32__ fix later may be easy.
>
>
>> +
>> +/* Structure to encapsulate all entities associated with pipe. ?*/
>> +
>> +struct pipe_obj
>> +{
>> + ?/* The delimiter to separate out gdb-command and shell-command. ?This can be
>> + ? ? any arbitrary string without containing any whitespace. ?*/
>> + ?char *dlim;
>> +
>> + ?/* The gdb-command. ?*/
>> + ?char *gdb_cmd;
>> +
>> + ?/* The shell-command. ?*/
>> + ?char *shell_cmd;
>> +
>> + ?/* The gdb-side stream pointer to the pipe. ?*/
>> + ?FILE *handle;
>> +
>> + ?/* The pex object used to create pipeline between gdb and shell. ?*/
>> + ?struct pex_obj *pex;
>> +};
>> +
>> +/* Destruct pipe object referenced by ARG. ?*/
>> +
>> +static void
>> +destruct_pipe (void *arg)
>> +{
>> + ?struct pipe_obj *pipe = (struct pipe_obj *) arg;
>> +
>> + ?if (pipe->handle != NULL)
>> + ? ?fclose (pipe->handle);
>> +
>> + ?if (pipe->pex != NULL)
>> + ? ?{
>> + ? ? ?int status;
>> +
>> + ? ? ?/* Wait till the process on the other side of the pipe completes its
>> + ? ? ?job before closing its file descriptors. ?*/
>> + ? ? ?pex_get_status (pipe->pex, 1, &status);
>> +
>> + ? ? ?if (!WIFEXITED (status))
>> + ? ? warning (_("Execution of shell-command `%s' may not be completed"),
>> + ? ? ? ? ? ? ?pipe->shell_cmd);
>
> WEXITSTATUS could be checked but you may not agree, OK if so.
>
>
>> +
>> + ? ? ?pex_free (pipe->pex);
>> + ? ?}
>> +
>> + ?xfree (pipe->dlim);
>> + ?xfree (pipe);
>> +}
>> +
>> +/* Construct a pipe object by parsing argument ARG to the pipe command. ?*/
>> +
>> +static struct pipe_obj *
>> +construct_pipe (char *arg)
>
> ARG can be now `const char *' when it is xstrdup-ed anyway.
>
>> +{
>> + ?char *p, *t;
>> + ?struct pipe_obj *pipe = NULL;
>> + ?struct cleanup *cleanup;
>> +
>> + ?if (arg == NULL)
>> + ? ?error (_("No argument is specified"));
>> +
>> + ?pipe = XCNEW (struct pipe_obj);
>> + ?cleanup = make_cleanup (destruct_pipe, pipe);
>> +
>> + ?p = xstrdup (arg);
>> + ?pipe->dlim = p;
>> +
>> + ?if (*pipe->dlim == '-')
>> + ? ?error (_("Delimiter pattern should not start with '-'"));
>> +
>> + ?t = skip_to_space (p);
>> + ?p = skip_spaces (t);
>> +
>> + ?if (*p == '\0')
>> + ? ?error (_("No gdb-command is specified"));
>> +
>> + ?*t = '\0';
>> + ?pipe->gdb_cmd = p;
>> +
>> + ?for (;;)
>> + ? ?{
>> + ? ? ?t = skip_to_space (p);
>> +
>> + ? ? ?if (*t == '\0')
>> + ? ? error (_("No shell-command is specified"));
>> +
>> + ? ? ?/* Check whether the token separated by whitespace matches with
>> + ? ? ?delimiter. ?*/
>> + ? ? ?if (memcmp (p, pipe->dlim, (t - p)) == 0
>> + ? ? ? && pipe->dlim[t - p] == '\0')
>> + ? ? {
>> + ? ? ? *p = '\0';
>> + ? ? ? pipe->shell_cmd = skip_spaces (t);
>> + ? ? ? break;
>> + ? ? }
>> +
>> + ? ? ? p = skip_spaces (t);
>> + ? ?}
>> +
>> + ?discard_cleanups (cleanup);
>> + ?return pipe;
>> +}
>> +
>> +/* Run execute_command for PIPE and FROM_TTY. ?Write output to the pipe, do not
>> + ? display it to the screen. ?*/
>> +
>> +static void
>> +execute_command_to_pipe (struct pipe_obj *pipe, int from_tty)
>> +{
>> + ?char *argv[4];
>> + ?struct cleanup *cleanup;
>> + ?struct ui_file *fp;
>> + ?int status;
>> + ?const char *errmsg;
>> + ?volatile struct gdb_exception exception;
>> +
>> + ?argv[0] = SHELL;
>> + ?argv[1] = OPTION_TO_SHELL;
>> + ?argv[2] = pipe->shell_cmd;
>> + ?argv[3] = NULL;
>> +
>> + ?pipe->pex = pex_init (PEX_USE_PIPES, argv[0], NULL);
>> + ?pipe->handle = pex_input_pipe (pipe->pex, 0);
>> +
>> + ?if (pipe->handle == NULL)
>> + ? ?error (_("Failed to create pipe"));
>> +
>> + ?errmsg = pex_run (pipe->pex, PEX_LAST, argv[0], argv, NULL, NULL, &status);
>> +
>> + ?if (errmsg != NULL)
>> + ? ?error (_("Failed to execute shell-command `%s' on pipe: %s"),
>> + ? ? ? ?pipe->shell_cmd, errmsg);
>
> Also output: safe_strerror (status)
>
>
>> +
>> + ?/* GDB_STDOUT should be better already restored during these
>> + ? ? restoration callbacks. ?*/
>> + ?cleanup = set_batch_flag_and_make_cleanup_restore_page_info ();
>> + ?fp = stdio_fileopen (pipe->handle);
>> + ?make_cleanup_ui_file_delete (fp);
>> + ?make_cleanup_restore_ui_file (&gdb_stdout);
>> + ?make_cleanup_restore_ui_file (&gdb_stderr);
>> + ?make_cleanup_restore_ui_file (&gdb_stdlog);
>> + ?make_cleanup_restore_ui_file (&gdb_stdtarg);
>> + ?make_cleanup_restore_ui_file (&gdb_stdtargerr);
>> +
>> + ?if (ui_out_redirect (current_uiout, fp) < 0)
>> + ? ?warning (_("Current output protocol does not support redirection"));
>> + ?else
>> + ? ?make_cleanup_ui_out_redirect_pop (current_uiout);
>> +
>> + ?gdb_stdout = fp;
>> + ?gdb_stderr = fp;
>> + ?gdb_stdlog = fp;
>> + ?gdb_stdtarg = fp;
>> + ?gdb_stdtargerr = fp;
>> +
>> + ?TRY_CATCH (exception, RETURN_MASK_ERROR)
>> + ? ?{
>> + ? ? ?execute_command (pipe->gdb_cmd, from_tty);
>> + ? ?}
>> +
>> + ?if (exception.reason < 0)
>> + ? ?exception_print (gdb_stderr, exception);
>> +
>> + ?do_cleanups (cleanup);
>> +}
>> +
>> +/* Execute the pipe command with argument ARG and FROM_TTY. ?*/
>> +
>> +static void
>> +pipe_command (char *arg, int from_tty)
>> +{
>> + ?struct pipe_obj *pipe = construct_pipe (arg);
>> + ?struct cleanup *cleanup = make_cleanup (destruct_pipe, pipe);
>> +
>> + ?execute_command_to_pipe (pipe, from_tty);
>> + ?do_cleanups (cleanup);
>> +}
>> +
>> +/* Module initialization. ?*/
>> +
>> +void
>> +_initialize_pipe (void)
>> +{
>> + ?add_cmd ("pipe", no_class, pipe_command, _("\
>> +Create pipe to pass gdb-command output to the shell for processing.\n\
>> +Arguments are a delimiter, followed by a gdb-command, then the same delimiter \
>> +again and finally a shell-command."),
>> + ? ? ? ?&cmdlist);
>> +}
>
>
> But otherwise OK to check it in with those few changes, approved doc and the
> testcase.
>
Sorry for bit delay in response. I am working on the doc part and
making corrections as suggested. At the same making progress on
copyright assignment (it may take few days). Thanks a lot for
reviewing this piece of code.
>
> Thanks,
> Jan
>