GNU bug report logs - #35046
[PATCH] factor: check the standard output instead of the standard input

Previous Next

Package: coreutils;

Reported by: Shugo Maeda <shugo <at> ruby-lang.org>

Date: Sat, 30 Mar 2019 08:31:02 UTC

Severity: normal

Tags: patch

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35046 in the body.
You can then email your comments to 35046 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-coreutils <at> gnu.org:
bug#35046; Package coreutils. (Sat, 30 Mar 2019 08:31:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Shugo Maeda <shugo <at> ruby-lang.org>:
New bug report received and forwarded. Copy sent to bug-coreutils <at> gnu.org. (Sat, 30 Mar 2019 08:31:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Shugo Maeda <shugo <at> ruby-lang.org>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] factor: check the standard output instead of the standard
 input
Date: Sat, 30 Mar 2019 15:43:17 +0900
* src/factor.c (lbuf_putc): Use line buffered mode if the standard
output (not the standard input) is a terminal in the same way as the
stdio library.  User programs might use pty only for the standard out
like the example of Ruby's PTY module:
https://docs.ruby-lang.org/en/2.6.0/PTY.html#module-PTY-label-Example
---
 src/factor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/factor.c b/src/factor.c
index 39e8918ff..173774045 100644
--- a/src/factor.c
+++ b/src/factor.c
@@ -2403,7 +2403,7 @@ lbuf_putc (char c)
       /* Provide immediate output for interactive input.  */
       static int line_buffered = -1;
       if (line_buffered == -1)
-        line_buffered = isatty (STDIN_FILENO);
+        line_buffered = isatty (STDOUT_FILENO);
       if (line_buffered)
         lbuf_flush ();
       else if (buffered >= FACTOR_PIPE_BUF)
-- 
2.11.0





Information forwarded to bug-coreutils <at> gnu.org:
bug#35046; Package coreutils. (Sat, 30 Mar 2019 21:59:02 GMT) Full text and rfc822 format available.

Message #8 received at 35046 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Shugo Maeda <shugo <at> ruby-lang.org>, 35046 <at> debbugs.gnu.org
Subject: Re: bug#35046: [PATCH] factor: check the standard output instead of
 the standard input
Date: Sat, 30 Mar 2019 14:58:52 -0700
On 29/03/19 23:43, Shugo Maeda wrote:
> * src/factor.c (lbuf_putc): Use line buffered mode if the standard
> output (not the standard input) is a terminal in the same way as the
> stdio library.  User programs might use pty only for the standard out
> like the example of Ruby's PTY module:
> https://docs.ruby-lang.org/en/2.6.0/PTY.html#module-PTY-label-Example
> ---
>  src/factor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/factor.c b/src/factor.c
> index 39e8918ff..173774045 100644
> --- a/src/factor.c
> +++ b/src/factor.c
> @@ -2403,7 +2403,7 @@ lbuf_putc (char c)
>        /* Provide immediate output for interactive input.  */
>        static int line_buffered = -1;
>        if (line_buffered == -1)
> -        line_buffered = isatty (STDIN_FILENO);
> +        line_buffered = isatty (STDOUT_FILENO);
>        if (line_buffered)
>          lbuf_flush ();
>        else if (buffered >= FACTOR_PIPE_BUF)

That would impact use cases like:

  factor | sed -u 's/.*: *//'

Do you have a use case like:

  slow_generator | factor

To support both we could line buffer if either stdin/out is a tty.
To support the following we'd need a flag:

  slow_generator | factor | sed -u 's/.*: *//'

cheers,
Pádraig




Information forwarded to bug-coreutils <at> gnu.org:
bug#35046; Package coreutils. (Sun, 31 Mar 2019 05:15:01 GMT) Full text and rfc822 format available.

Message #11 received at 35046 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>,
 Shugo Maeda <shugo <at> ruby-lang.org>, 35046 <at> debbugs.gnu.org
Subject: Re: bug#35046: [PATCH] factor: check the standard output instead of
 the standard input
Date: Sat, 30 Mar 2019 22:14:36 -0700
Pádraig Brady wrote:
>> -        line_buffered = isatty (STDIN_FILENO);
>> +        line_buffered = isatty (STDOUT_FILENO);
>>         if (line_buffered)
>>           lbuf_flush ();
>>         else if (buffered >= FACTOR_PIPE_BUF)
> That would impact use cases like:
> 
>    factor | sed -u 's/.*: *//'

Sure, but the longstanding tradition for almost all GNU utilities is for stdout 
to be line-buffered if it is a tty (regardless of whether stdin is a tty), and 
that's how 'factor' behaved until we started fooling with buffering in coreutils 
8.24. Shouldn't we simply go back to the longstanding tradition for deciding 
whether to line-buffer stdout? Surely factor's current behavior is simply a bug 
introduced in 8.24 and not fixed in 8.26.




Reply sent to Pádraig Brady <P <at> draigBrady.com>:
You have taken responsibility. (Sun, 31 Mar 2019 22:54:03 GMT) Full text and rfc822 format available.

Notification sent to Shugo Maeda <shugo <at> ruby-lang.org>:
bug acknowledged by developer. (Sun, 31 Mar 2019 22:54:03 GMT) Full text and rfc822 format available.

Message #16 received at 35046-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Shugo Maeda <shugo <at> ruby-lang.org>,
 35046-done <at> debbugs.gnu.org
Subject: Re: bug#35046: [PATCH] factor: check the standard output instead of
 the standard input
Date: Sun, 31 Mar 2019 15:53:44 -0700
On 30/03/19 22:14, Paul Eggert wrote:
> Pádraig Brady wrote:
>>> -        line_buffered = isatty (STDIN_FILENO);
>>> +        line_buffered = isatty (STDOUT_FILENO);
>>>         if (line_buffered)
>>>           lbuf_flush ();
>>>         else if (buffered >= FACTOR_PIPE_BUF)
>> That would impact use cases like:
>>
>>    factor | sed -u 's/.*: *//'
> 
> Sure, but the longstanding tradition for almost all GNU utilities is for stdout 
> to be line-buffered if it is a tty (regardless of whether stdin is a tty), and 
> that's how 'factor' behaved until we started fooling with buffering in coreutils 
> 8.24. Shouldn't we simply go back to the longstanding tradition for deciding 
> whether to line-buffer stdout? Surely factor's current behavior is simply a bug 
> introduced in 8.24 and not fixed in 8.26.

I should be clearer, sorry.

I think this could be improved.
I've pushed the change to set line_buffered if either stdin or stdout isatty():
https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=6d78a28

I was wondering if there was a practical use case that this impacted.
Enough justification was provided in the commit message which I didn't read fully.

The original buffering adjustments were so that parallel invocations of factor,
output lines atomically, which factor is able to achieve given its restricted output format.

Marking this as done.

thanks,
Pádraig




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 29 Apr 2019 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 335 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.