Re: [PATCH] Supress SIGTTOU when handling errors

[Pedro Alves <palves@redhat.com>]

On 5/24/19 9:54 AM, Alan Hayward wrote:

> Looking back at my original patch again, I’m wondering if it’s better to
> move the ignore higher up the call stack in print_flush, so that it’s set
> across both flushes:

What are the two flushes?  I only see one, from the serial_drain_output call?

In any case, I think it's better to keep the SIGTTOU handling close to
the tcdrain call, to make to code a lot more obvious -- SIGTTOU suppression
is described in tcdrain manuals -- and I don't think we have to worry
about efficiency here?

> ...or if it really should be left just around the tcdrain.
> Not quite sure what the behaviour is on non-Linux targets.

The behavior should be the same on all POSIX systems:

https://pubs.opengroup.org/onlinepubs/009695399/functions/tcdrain.html

 "Any attempts to use tcdrain() from a process which is a member of a background
 process group on a fildes associated with its controlling terminal, shall cause the process
 group to be sent a SIGTTOU signal. If the calling process is blocking or ignoring
 SIGTTOU signals, the process shall be allowed to perform the operation, and
 no signal is sent."

On non-POSIX systems, the serial_drain_output call doesn't end up in
ser-unix.c:hardwire_drain_output, so from that perspective, putting
the SIGTTOU suppression in common code is a bit of an abstraction violation.

> 
> 
>>
>> That isn't to say that your patch _isn't_ also correct.  We have many
>> latent bugs around this area.  Let me take a better look at that one too.
>>
>> I think that even if we get something like your patch in, then
>> Alan's is still correct because we can have places doing
>> try/catch to swallow an error but still print it with exception_print,
>> all while the inferior is running.  Of course such spots should
>> call ours_for_output(), but that will run into the tcdrain issue.
>>
> 
> Minor issue is that once my patch is in, it’ll hide the missing ours_for_output
> bugs (?)
> 

Sure.  But we shouldn't avoid fixing one bug because of that.  The
palves/tty-always-separate-session branch on my github exposes such
bugs because with a missing ours_for_output call, gdb prints to
the screen while the terminal is in raw mode, resulting in output
like

this is line one
                this is line two
                                this is line three

instead of:

this is line one
this is line two
this is line three

Thanks,
Pedro Alves