From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2908 invoked by alias); 8 Jan 2012 07:27:02 -0000 Received: (qmail 2896 invoked by uid 22791); 8 Jan 2012 07:27:00 -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-ww0-f43.google.com (HELO mail-ww0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 08 Jan 2012 07:26:47 +0000 Received: by wgbdt11 with SMTP id dt11so569491wgb.12 for ; Sat, 07 Jan 2012 23:26:45 -0800 (PST) MIME-Version: 1.0 Received: by 10.180.77.43 with SMTP id p11mr6321826wiw.17.1326007605629; Sat, 07 Jan 2012 23:26:45 -0800 (PST) Received: by 10.180.107.69 with HTTP; Sat, 7 Jan 2012 23:26:45 -0800 (PST) In-Reply-To: References: Date: Sun, 08 Jan 2012 07:27: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/msg00259.txt.bz2 On Sun, Jan 8, 2012 at 12:46 PM, Sergio Durigan Junior wrote: > 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. > > Yeah, I am not sure we should worry about this, it seems to be very > specific indeed. > >>> >>>> +/* Structure to encapsulate all entities associated with pipe. =A0*/ >>>> + >>>> +struct pipe_obj >>>> +{ >>>> + =A0/* The delimiter to separate out gdb-command and shell-command. = =A0This can be >>>> + =A0 =A0 any arbitrary string without containing any whitespace. =A0T= here 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. > > Unfortunately GDB and others are full of those discrepancies. =A0Thanks > for letting me know that this const-correctness is actually harder than > I thought. =A0In my opinion, you should not worry about this for now > otherwise your work will be compromised in a snowball of problems. =A0I > myself am facing such problems in another patch of mine. =A0So I think I > will just take back what I said here. > > Since I am not a maintainer, I cannot approve your patch, so I will take > some time tomorrow and test it further while others review it. > > Thanks for working on this. Thanks for your time in reviewing this. Regards, Abhijit Halder