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

Jan Kratochvil
2011-08-20T19:16:17Z

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.

Thanks,
Jan