From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26682 invoked by alias); 8 Jan 2012 06:57:20 -0000 Received: (qmail 26670 invoked by uid 22791); 8 Jan 2012 06:57:19 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_RG X-Spam-Check-By: sourceware.org Received: from mail-we0-f169.google.com (HELO mail-we0-f169.google.com) (74.125.82.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 08 Jan 2012 06:57:06 +0000 Received: by werf1 with SMTP id f1so2576835wer.0 for ; Sat, 07 Jan 2012 22:57:05 -0800 (PST) MIME-Version: 1.0 Received: by 10.216.138.201 with SMTP id a51mr6321694wej.24.1326005824970; Sat, 07 Jan 2012 22:57:04 -0800 (PST) Received: by 10.180.107.69 with HTTP; Sat, 7 Jan 2012 22:57:04 -0800 (PST) In-Reply-To: References: Date: Sun, 08 Jan 2012 06:57:00 -0000 Message-ID: Subject: Re: [PATCH] Implementation of pipe to pass GDB's command output to the shell. From: Abhijit Halder To: Sergio Durigan Junior Cc: "gdb-patches@sourceware.org ml" 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: 2012-01/txt/msg00257.txt.bz2 On Sat, Jan 7, 2012 at 11:49 PM, Sergio Durigan Junior wrote: > Hey Abhijit, > > Thanks for keep trying to push this upstream. Thanks for your encouragement. I would love to see this feature in next GDB release! > Only a few comments. > > Abhijit Halder writes: > >> +#if defined (__MINGW32__) >> +# define DEFAULT_SHELL "cmd.exe" >> +# define OPTION_TO_SHELL "/c" >> +#else >> +# define DEFAULT_SHELL "/bin/sh" >> +# define OPTION_TO_SHELL "-c" >> +#endif > > As far as I have researched, all bash-compatible shells accept `-c' as a > parameter, and all of them interpret this parameter in the same way > (i.e., "execute this command"). =A0However, and I am not sure this is > something we should worry about or not, there might be other shells > around which do not support `-c', or expect something else. =A0I don't > know if a check is worthwhile in this case. Actually I am not aware of this. But does GDB support such shell? If yes, then ofcourse we should put a check. Please comment further. > >> +/* 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. =A0The= re should not >> + =A0 =A0 be any leading '-' in the delimiter. =A0*/ >> + =A0char *dlim; > > I believe this can be declared as const, right? =A0Same thing for > `gdb_cmd' below. =A0Unfortunately, `shell_cmd' cannot be declared const > for now because is is using `skip_spaces', which does not accept a > `const char *' as argument. > > Otherwise, the patch looks good to me. I was very much tempted for making this as const char * and that's why submitted the patch in hurry! Actually pex_run expects argv to be char * const *. I was about to ask this question whether this is required? Can't we change the prototype of pex_run? Regarding use of shell_cmd here, we can typecast that thing. Only bottleneck is pex_run. There also typecast may work, atleast for compilation, but this can give runtime error in case we don't treat argv inside pex_run as an array of const string. Thanks, Abhijit Halder