[PATCH] An implementation of pipe to make I/O communication between gdb and shell.
Abhijit Halder
abhijit.k.halder@gmail.com
Mon Aug 8 11:15:00 GMT 2011
On Mon, Aug 8, 2011 at 3:30 PM, Abhijit Halder
<abhijit.k.halder@gmail.com> wrote:
> On Sun, Aug 7, 2011 at 3:50 AM, Sergio Durigan Junior
> <sergiodj@redhat.com> wrote:
>> Hi Abhijit,
>>
>> Some minor nits (again).
>>
>> Abhijit Halder <abhijit.k.halder@gmail.com> writes:
>>
>>> +struct pipe_obj
>>> +{
>>> + /* The delimiter to separate out gdb-command and shell-command. This can be
>> ^^
> Sorry, here I am not clear. Do you mean to say I should remove - in
> between gdb and command?
>>
>> Two spaces after the period.
>>
>>> + /* The pex object use to create pipeline between gdb and shell. */
>>
> Here I guess I have put 2 space character between . (period) and */.
>> Some typos:
>>
>> "/*The pex object used to create a pipeline between GDB and shell. */"
>>
>>> + if (*p != '\0') *p++ = '\0';
>>
>> I'm not found of this style. Please, put the assignment on the next
>> line:
> Yes a slip. I am modifying it.
>>
>> if (*p != '\0')
>> *p++ = '\0';
>>
>>
>>> + for (pipe->shell_cmd = "";
>>
>> Sorry, I didn't understand this line. Is this needed? If so, I think
>> it's better if you do `pipe->shell_cmd = NULL'.
>>
>>> + int mismatch = memcmp (p, pipe->dlim, (separator-p));
>>
>> Space between `separator', `-' and `p'. If you want, you can put this
>> `memcmp' call directly on the `if' condition, and then you wouldn't need
>> the braces on the `for'.
>>
> Modifying this part.
>>> +/* Run execute_command for P and FROM_TTY. Write output to the pipe, do not
>> ^^
>> Two spaces after period.
>>
>>> + if (pipe->handle == NULL)
>>> + error (_("Failed to create pipe"));
>>> +
>>> + {
>>> + int status;
>>> + const char *err
>>> + = pex_run (pipe->pex,
>>> + PEX_SEARCH | PEX_LAST | PEX_STDERR_TO_STDOUT,
>>> + argv[0], argv,
>>> + NULL, NULL,
>>> + &status);
>>> + if (err)
>>> + error (_("Failed to execute %s"), argv[0]);
>>> +
>>> + do_cleanups (cleanup);
>>> + }
>>
>> Why the braces?
> Since pex_run returns a const char * I have to assign err at declaration time.
>>
>>> + if (pipe->pex)
>>> + {
>>> + int status;
>>> +
>>> + pex_get_status (pipe->pex, 1, &status);
>>> + pex_free (pipe->pex);
>>
>> Do you really need to call `pex_get_status' here? I'm really asking,
>> because I don't know about pex.
> I tried without using pex_get_status, but pex_free is killing the child
> process without calling waitpid (or equivalent). This is causing an
> immediate exit of shell command. I may be missing something. Let me
> dig out further inside code.
Probably I got the answer here. In pex_free we close the std
descriptors before calling pex_get_status_and_time, hence we do not
get any output. I think we have to call pex_get_status before calling
pex_free.
>>
>> Thanks for your work on this patch.
>>
>> Regards,
>>
>> Sergio.
>>
>
> Finally thank you all for your valuable time, showing me directions
> and through review of the code. Shortly I will come back with
> suggested modifications.
>
> Regards,
> Abhijit Halder
>
More information about the Gdb-patches
mailing list