Re: [PATCH] Supress SIGTTOU when handling errors

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Alan Hayward <Alan.Hayward@arm.com> [2019-05-24 12:36:43 +0000]:

> 
> 
> > On 24 May 2019, at 12:02, Pedro Alves <palves@redhat.com> wrote:
> > 
> > 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?
> 
> Sorry, I was forgot it was that call. I was thinking it was as part of the
> gdb_flush 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?
> 
> True - errors from gdb shouldn’t be frequent enough to be an issue.
> 
> > 
> >> ...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.
> 
> Ok, agreed.
> 
> Any objections to me pushing the original patch then?

Not from me.

Sorry for any delay I caused in getting the patch merged.

Thanks,
Andrew


> 
> 
> > 
> >> 
> >> 
> >>> 
> >>> 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
>