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] Fix inferior deadlock with "target remote | CMD"


On 10/18/2017 06:38 PM, John Baldwin wrote:
> On Monday, October 16, 2017 08:44:41 PM Pedro Alves wrote:

> Seems sensible.  It wasn't obvious to me from the code how stdout is
> treated as compared to stderr, so I'm less confident on reviewing the
> implementation.  One nit:

Ah, the manual explains it:

~~~
Programs started with stdio-connected gdbserver have @file{/dev/null} for
@code{stdin}, and @code{stdout},@code{stderr} are sent back to gdb for
display through a pipe connected to gdbserver.
Both @code{stdout} and @code{stderr} use the same pipe.
~~~

> 
>> @@ -589,6 +602,10 @@ ser_base_async (struct serial *scb,
>>  	fprintf_unfiltered (gdb_stdlog, "[fd%d->asynchronous]\n",
>>  			    scb->fd);
>>        reschedule (scb);
>> +
>> +      if (scb->error_fd != -1)
>> +	add_file_handler (scb->error_fd, handle_error_fd, scb);
>> +
>>      }
> 
> Extra blank line at the end?

Indeed.  I fixed it.

> 
>> +
>> +set expected_lines 3000
> 
> Pity this can't be a function of PIPE_BUF, but I suspect there's
> no good way to do that.

Yeah, I don't know of a way.  Well, actually the test itself could
maybe create a pipe and try to fill it up, which would be the
right number when native testing.  That seemed more trouble than
it's worth it, though.

Looking around the web a bit more, I found
<https://github.com/afborchert/pipebuf> which confirms that 70KB
"ought to be enough for everybody".

I pushed the patch in now, also with filename filed in the ChangeLog.

Thanks for the review!

Pedro Alves


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