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

[Sergio Durigan Junior <sergiodj at redhat dot com>]

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.

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. Â*/
>>> +
>>> +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.

Unfortunately GDB and others are full of those discrepancies.  Thanks
for letting me know that this const-correctness is actually harder than
I thought.  In my opinion, you should not worry about this for now
otherwise your work will be compromised in a snowball of problems.  I
myself am facing such problems in another patch of mine.  So 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.