Re: [PATCH] Implementation of pipe to pass GDB's command output to the shell.

[Abhijit Halder <abhijit.k.halder@gmail.com>]

On Sat, Jan 7, 2012 at 11:49 PM, Sergio Durigan Junior
<sergiodj@redhat.com> 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 <abhijit.k.halder@gmail.com> 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").  However, 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.  I 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.  */
>> +
>> +struct pipe_obj
>> +{
>> +  /* The delimiter to separate out gdb-command and shell-command.  This can be
>> +     any arbitrary string without containing any whitespace.  There should not
>> +     be any leading '-' in the delimiter.  */
>> +  char *dlim;
>
> I believe this can be declared as const, right?  Same thing for
> `gdb_cmd' below.  Unfortunately, `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