From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6736 invoked by alias); 23 Aug 2011 06:26:17 -0000 Received: (qmail 6728 invoked by uid 22791); 23 Aug 2011 06:26:15 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_TD X-Spam-Check-By: sourceware.org Received: from mail-ey0-f169.google.com (HELO mail-ey0-f169.google.com) (209.85.215.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Aug 2011 06:25:52 +0000 Received: by eye22 with SMTP id 22so3531236eye.0 for ; Mon, 22 Aug 2011 23:25:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.213.29.76 with SMTP id p12mr920584ebc.74.1314080750981; Mon, 22 Aug 2011 23:25:50 -0700 (PDT) Received: by 10.213.33.142 with HTTP; Mon, 22 Aug 2011 23:25:50 -0700 (PDT) In-Reply-To: <20110820191535.GA7527@host1.jankratochvil.net> References: <20110813205053.GB22058@host1.jankratochvil.net> <20110814121407.GA29236@host1.jankratochvil.net> <83y5ywulun.fsf@gnu.org> <20110814170136.GA26819@host1.jankratochvil.net> <83sjp3vkze.fsf@gnu.org> <20110814195523.GA7588@host1.jankratochvil.net> <20110820191535.GA7527@host1.jankratochvil.net> Date: Tue, 23 Aug 2011 06:26:00 -0000 Message-ID: Subject: Re: [PATCH] An implementation of pipe to make I/O communication between gdb and shell. [MinGW question] From: Abhijit Halder To: Jan Kratochvil Cc: Eli Zaretskii , sergiodj@redhat.com, tromey@redhat.com, pedro@codesourcery.com, gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00415.txt.bz2 On Sun, Aug 21, 2011 at 12:45 AM, Jan Kratochvil wrote: > On Tue, 16 Aug 2011 14:45:19 +0200, Abhijit Halder wrote: >> +#include "defs.h" >> +#include >> +#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. =A0*/ >> + >> +struct pipe_obj >> +{ >> + =A0/* The delimiter to separate out gdb-command and shell-command. =A0= This can be >> + =A0 =A0 any arbitrary string without containing any whitespace. =A0*/ >> + =A0char *dlim; >> + >> + =A0/* The gdb-command. =A0*/ >> + =A0char *gdb_cmd; >> + >> + =A0/* The shell-command. =A0*/ >> + =A0char *shell_cmd; >> + >> + =A0/* The gdb-side stream pointer to the pipe. =A0*/ >> + =A0FILE *handle; >> + >> + =A0/* The pex object used to create pipeline between gdb and shell. = =A0*/ >> + =A0struct pex_obj *pex; >> +}; >> + >> +/* Destruct pipe object referenced by ARG. =A0*/ >> + >> +static void >> +destruct_pipe (void *arg) >> +{ >> + =A0struct pipe_obj *pipe =3D (struct pipe_obj *) arg; >> + >> + =A0if (pipe->handle !=3D NULL) >> + =A0 =A0fclose (pipe->handle); >> + >> + =A0if (pipe->pex !=3D NULL) >> + =A0 =A0{ >> + =A0 =A0 =A0int status; >> + >> + =A0 =A0 =A0/* Wait till the process on the other side of the pipe comp= letes its >> + =A0 =A0 =A0job before closing its file descriptors. =A0*/ >> + =A0 =A0 =A0pex_get_status (pipe->pex, 1, &status); >> + >> + =A0 =A0 =A0if (!WIFEXITED (status)) >> + =A0 =A0 warning (_("Execution of shell-command `%s' may not be complet= ed"), >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0pipe->shell_cmd); > > WEXITSTATUS could be checked but you may not agree, OK if so. > > >> + >> + =A0 =A0 =A0pex_free (pipe->pex); >> + =A0 =A0} >> + >> + =A0xfree (pipe->dlim); >> + =A0xfree (pipe); >> +} >> + >> +/* Construct a pipe object by parsing argument ARG to the pipe command.= =A0*/ >> + >> +static struct pipe_obj * >> +construct_pipe (char *arg) > > ARG can be now `const char *' when it is xstrdup-ed anyway. > >> +{ >> + =A0char *p, *t; >> + =A0struct pipe_obj *pipe =3D NULL; >> + =A0struct cleanup *cleanup; >> + >> + =A0if (arg =3D=3D NULL) >> + =A0 =A0error (_("No argument is specified")); >> + >> + =A0pipe =3D XCNEW (struct pipe_obj); >> + =A0cleanup =3D make_cleanup (destruct_pipe, pipe); >> + >> + =A0p =3D xstrdup (arg); >> + =A0pipe->dlim =3D p; >> + >> + =A0if (*pipe->dlim =3D=3D '-') >> + =A0 =A0error (_("Delimiter pattern should not start with '-'")); >> + >> + =A0t =3D skip_to_space (p); >> + =A0p =3D skip_spaces (t); >> + >> + =A0if (*p =3D=3D '\0') >> + =A0 =A0error (_("No gdb-command is specified")); >> + >> + =A0*t =3D '\0'; >> + =A0pipe->gdb_cmd =3D p; >> + >> + =A0for (;;) >> + =A0 =A0{ >> + =A0 =A0 =A0t =3D skip_to_space (p); >> + >> + =A0 =A0 =A0if (*t =3D=3D '\0') >> + =A0 =A0 error (_("No shell-command is specified")); >> + >> + =A0 =A0 =A0/* Check whether the token separated by whitespace matches = with >> + =A0 =A0 =A0delimiter. =A0*/ >> + =A0 =A0 =A0if (memcmp (p, pipe->dlim, (t - p)) =3D=3D 0 >> + =A0 =A0 =A0 && pipe->dlim[t - p] =3D=3D '\0') >> + =A0 =A0 { >> + =A0 =A0 =A0 *p =3D '\0'; >> + =A0 =A0 =A0 pipe->shell_cmd =3D skip_spaces (t); >> + =A0 =A0 =A0 break; >> + =A0 =A0 } >> + >> + =A0 =A0 =A0 p =3D skip_spaces (t); >> + =A0 =A0} >> + >> + =A0discard_cleanups (cleanup); >> + =A0return pipe; >> +} >> + >> +/* Run execute_command for PIPE and FROM_TTY. =A0Write output to the pi= pe, do not >> + =A0 display it to the screen. =A0*/ >> + >> +static void >> +execute_command_to_pipe (struct pipe_obj *pipe, int from_tty) >> +{ >> + =A0char *argv[4]; >> + =A0struct cleanup *cleanup; >> + =A0struct ui_file *fp; >> + =A0int status; >> + =A0const char *errmsg; >> + =A0volatile struct gdb_exception exception; >> + >> + =A0argv[0] =3D SHELL; >> + =A0argv[1] =3D OPTION_TO_SHELL; >> + =A0argv[2] =3D pipe->shell_cmd; >> + =A0argv[3] =3D NULL; >> + >> + =A0pipe->pex =3D pex_init (PEX_USE_PIPES, argv[0], NULL); >> + =A0pipe->handle =3D pex_input_pipe (pipe->pex, 0); >> + >> + =A0if (pipe->handle =3D=3D NULL) >> + =A0 =A0error (_("Failed to create pipe")); >> + >> + =A0errmsg =3D pex_run (pipe->pex, PEX_LAST, argv[0], argv, NULL, NULL,= &status); >> + >> + =A0if (errmsg !=3D NULL) >> + =A0 =A0error (_("Failed to execute shell-command `%s' on pipe: %s"), >> + =A0 =A0 =A0 =A0pipe->shell_cmd, errmsg); > > Also output: safe_strerror (status) > > >> + >> + =A0/* GDB_STDOUT should be better already restored during these >> + =A0 =A0 restoration callbacks. =A0*/ >> + =A0cleanup =3D set_batch_flag_and_make_cleanup_restore_page_info (); >> + =A0fp =3D stdio_fileopen (pipe->handle); >> + =A0make_cleanup_ui_file_delete (fp); >> + =A0make_cleanup_restore_ui_file (&gdb_stdout); >> + =A0make_cleanup_restore_ui_file (&gdb_stderr); >> + =A0make_cleanup_restore_ui_file (&gdb_stdlog); >> + =A0make_cleanup_restore_ui_file (&gdb_stdtarg); >> + =A0make_cleanup_restore_ui_file (&gdb_stdtargerr); >> + >> + =A0if (ui_out_redirect (current_uiout, fp) < 0) >> + =A0 =A0warning (_("Current output protocol does not support redirectio= n")); >> + =A0else >> + =A0 =A0make_cleanup_ui_out_redirect_pop (current_uiout); >> + >> + =A0gdb_stdout =3D fp; >> + =A0gdb_stderr =3D fp; >> + =A0gdb_stdlog =3D fp; >> + =A0gdb_stdtarg =3D fp; >> + =A0gdb_stdtargerr =3D fp; >> + >> + =A0TRY_CATCH (exception, RETURN_MASK_ERROR) >> + =A0 =A0{ >> + =A0 =A0 =A0execute_command (pipe->gdb_cmd, from_tty); >> + =A0 =A0} >> + >> + =A0if (exception.reason < 0) >> + =A0 =A0exception_print (gdb_stderr, exception); >> + >> + =A0do_cleanups (cleanup); >> +} >> + >> +/* Execute the pipe command with argument ARG and FROM_TTY. =A0*/ >> + >> +static void >> +pipe_command (char *arg, int from_tty) >> +{ >> + =A0struct pipe_obj *pipe =3D construct_pipe (arg); >> + =A0struct cleanup *cleanup =3D make_cleanup (destruct_pipe, pipe); >> + >> + =A0execute_command_to_pipe (pipe, from_tty); >> + =A0do_cleanups (cleanup); >> +} >> + >> +/* Module initialization. =A0*/ >> + >> +void >> +_initialize_pipe (void) >> +{ >> + =A0add_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 del= imiter \ >> +again and finally a shell-command."), >> + =A0 =A0 =A0 =A0&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 >