Bug 222671 - tail(1): tail -r fails on certain piped input
Summary: tail(1): tail -r fails on certain piped input
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 11.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL:
Keywords: patch, regression
Depends on:
Blocks:
 
Reported: 2017-09-28 22:51 UTC by Jim Long
Modified: 2018-04-16 16:44 UTC (History)
3 users (show)

See Also:
asomers: mfc-stable10+
asomers: mfc-stable11+


Attachments
Fix "tail -r" for pipes that begin with a newline (2.12 KB, patch)
2018-02-19 17:43 UTC, Alan Somers
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jim Long 2017-09-28 22:51:35 UTC
I'm not 100% sure of the conditions, but it appears that when:

1) tail -r is reading from a pipe;

2) the first input on the pipe is a newline (first line is blank)

tail will reverse the output, except it gets the Nth and (N-1)th output lines
wrong.  It places the first line of input (the newline) on the (N-1)th line of output, and it place the second line of input on the Nth line of output.

$ uname -a
FreeBSD g5.umpquanet.com 11.1-STABLE FreeBSD 11.1-STABLE #0 r321722: Sun Jul 30 12:07:20 PDT 2017     root@g5.umpquanet.com:/usr/obj/usr/src/sys/G5  amd64
$ printf '\n1\n'

1
$ printf '\n1\n' | tail -r

1
$ printf '\n1\n2\n3\n'

1
2
3
$ printf '\n1\n2\n3\n' | tail -r
3
2

1
$ 

I looked at the GitHub mirror and downloaded a few src tarballs around recent commits to tail.

Building tail from:

https://github.com/freebsd/freebsd/tree/97a2ca501c7e9f07a80339a88fbb5a46977ae428

does not manifest the bug.

However, tail built from:

https://github.com/freebsd/freebsd/tree/2fef72832029bcc228a527dd9c423ba34a7490e9

does.  IANAE, but it appears the bug may have crept in during:

https://reviews.freebsd.org/D9067


HTH,

Jim
Comment 1 Jim Long 2017-09-28 23:27:27 UTC
> ... tail will reverse the output ...

s/b

... tail will reverse the input ...


https://github.com/freebsd/freebsd/tree/97a2ca501c7e9f07a80339a88fbb5a46977ae428

(latest known absence of the bug) is circa January 4, 2017.


https://github.com/freebsd/freebsd/tree/2fef72832029bcc228a527dd9c423ba34a7490e9

(earliest known occurrence) is circa January 10, 2017.
Comment 2 pprocacci 2018-02-19 08:15:36 UTC
The problem is in usr.bin/tail/reverse.c line 258.

The variable `start` is only true when the pointer is at the beginning of the buffer.  *p is never checked to see if it is a newline while `start` is true.

This has the effect of the following being written:  '\n1\n'.
It does appear that the newline is randomly inserted, but this isn't the case.
It's essentially writing this:

write(1, '2\n', 2);
write(1, '\n1\n', 3);

Excuse the inline patch (too late in the morning to create an attachement).
The patch solves the problem by checking *p when start is true and fixes the problem.


--- reverse.c.orig      2018-02-19 03:13:53.835943000 -0500
+++ reverse.c   2018-02-19 03:14:34.665507000 -0500
@@ -255,8 +255,12 @@
                        if ((*p == '\n') || start) {
                                struct bfelem *tr;

-                               if (start && llen)
-                                       WR(p, llen + 1);
+                               if (start && llen){
+                                       if(*p == '\n')
+                                               WR(p + 1, llen);
+                                       else
+                                               WR(p, llen + 1);
+                               }
                                else if (llen)
                                        WR(p + 1, llen);
                                tr = TAILQ_NEXT(tl, entries);
Comment 3 Alan Somers freebsd_committer 2018-02-19 17:43:01 UTC
Created attachment 190812 [details]
Fix "tail -r" for pipes that begin with a newline

Try this patch instead.  If neither of you provide feedback, I'll commit it by the end of the week.
Comment 4 pprocacci 2018-02-19 18:51:25 UTC
Your patch is working fine.  In fact better as that was another thing I was going to mention, but didn't have the time to address; that was the stray newline being ignored and not written.

+1 from me.
Comment 5 Jim Long 2018-02-19 22:02:56 UTC
Thank you!

I presume this patch is against HEAD?  I excerpted just the edit to reverse.c, and applied it to 11.1-STABLE:

# grep DID reverse.c
__FBSDID("$FreeBSD: stable/11/usr.bin/tail/reverse.c 314427 2017-02-28 22:49:41Z asomers $");
# patch < /tmp/patch.1
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: usr.bin/tail/reverse.c
|===================================================================
|--- usr.bin/tail/reverse.c     (revision 329580)
|+++ usr.bin/tail/reverse.c     (working copy)
--------------------------
Patching file reverse.c using Plan A...
Hunk #1 succeeded at 255 (offset -2 lines).
done

It built fine and passed all my test cases.

Please advise or update this PR when this gets MFC'ed.

Thanks again to all involved.
Comment 6 commit-hook freebsd_committer 2018-02-19 22:10:02 UTC
A commit references this bug:

Author: asomers
Date: Mon Feb 19 22:09:49 UTC 2018
New revision: 329606
URL: https://svnweb.freebsd.org/changeset/base/329606

Log:
  tail: fix "tail -r" for piped input that begins with '\n'

  A subtle logic bug, probably introduced in r311895, caused tail to print the
  first two lines of piped input in forward order, if the very first character
  was a newline.

  PR:		222671
  Reported by:	Jim Long <freebsd-bugzilla@umpquanet.com>, pprocacci@gmail.com
  MFC after:	3 weeks
  Sponsored by:	Spectra Logic Corp

Changes:
  head/usr.bin/tail/reverse.c
  head/usr.bin/tail/tests/tail_test.sh
Comment 7 commit-hook freebsd_committer 2018-04-16 16:21:16 UTC
A commit references this bug:

Author: asomers
Date: Mon Apr 16 16:20:39 UTC 2018
New revision: 332600
URL: https://svnweb.freebsd.org/changeset/base/332600

Log:
  MFC r329606:

  tail: fix "tail -r" for piped input that begins with '\n'

  A subtle logic bug, probably introduced in r311895, caused tail to print the
  first two lines of piped input in forward order, if the very first character
  was a newline.

  PR:		222671
  Reported by:	Jim Long <freebsd-bugzilla@umpquanet.com>, pprocacci@gmail.com
  Sponsored by:	Spectra Logic Corp

Changes:
_U  stable/11/
  stable/11/usr.bin/tail/reverse.c
  stable/11/usr.bin/tail/tests/tail_test.sh
Comment 8 commit-hook freebsd_committer 2018-04-16 16:42:54 UTC
A commit references this bug:

Author: asomers
Date: Mon Apr 16 16:42:17 UTC 2018
New revision: 332610
URL: https://svnweb.freebsd.org/changeset/base/332610

Log:
  MFC r329606:

  tail: fix "tail -r" for piped input that begins with '\n'

  A subtle logic bug, probably introduced in r311895, caused tail to print the
  first two lines of piped input in forward order, if the very first character
  was a newline.

  PR:		222671
  Reported by:	Jim Long <freebsd-bugzilla@umpquanet.com>, pprocacci@gmail.com
  Sponsored by:	Spectra Logic Corp

Changes:
_U  stable/10/
  stable/10/usr.bin/tail/reverse.c
  stable/10/usr.bin/tail/tests/tail_test.sh