This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]