On Thu, Aug 4, 2011 at 2:59 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 04 August 2011 08:51:14, Abhijit Halder wrote:
>> +  fstream = gdb_modify_io (gdb_stdio, pstream);
>> +  execute_command (pipe->gdb_cmd, from_tty);
>> +  pstream = gdb_modify_io (gdb_stdio, fstream);
>
> Looks like this leaves gdb_stdio in an inconsistent
> state if execute_command throws an error.
>
> Do you really need the new gdb_modify_io function?
> Can't ui_out_redirect (and stdio_file_new perhaps) do the job?
>
Yes you are right. I could not foresee this.

>> +      pipe->handle = popen (pipe->shell_cmd, pipe->mode);
>
> I'm not sure that'll build on all supported hosts.
> I think on Windows that may require use of _popen instead.
>
> "struct pipe_t" sounds like a recipe for system namespace
> colision (and _t is reserved for posix, though we
> have some precedent for abusing it), and is easily confused
> with the ser*.c pipe machinery.  Can you find an
> alternative name for the struct please?
> Perhaps struct pipe_cmd_state.
>
> --
> Pedro Alves
>

I have made the suggested corrections. Please review this.

Thanks,
Abhijit Halder