On Fri, Jan 08, 2021 at 07:09:01PM -0800, Jordan Geoghegan wrote:
> Hey folks,
> 
> I've noticed some surprising behaviour from cmp(1) when using the '-s'
> flag.
> 
> It appears that cmp -s is ignoring the byte offset arguments I'm giving
> it.
<snip>
> Not sure what to make of this, I noticed this same behaviour on
> DragonflyBSD and FreeBSD, so maybe I'm just missing something obvious.
> This certainly caused some frustration before I figured out what was going
> on.

The bug seems to be in the short-circuit optimization for regular files[1]:

    void
  c_regular(int fd1, char *file1, off_t skip1, off_t len1,
      int fd2, char *file2, off_t skip2, off_t len2)
  {
        u_char ch, *p1, *p2;
        off_t byte, length, line;
        int dfound;
  
        if (sflag && len1 != len2)
                exit(1);
  
        if (skip1 > len1)
                eofmsg(file1);
        len1 -= skip1;
        if (skip2 > len2)
                eofmsg(file2);
        len2 -= skip2;

The short-circuit should probably be moved below the subsequent chunk of
code (i.e. below `len2 -= skip2`). The eofmsg function already obeys sflag,
so it'll be quiet.[2] Doing this works for me. See patch at end of message.

Interestingly, DragonflyBSD and FreeBSD already do it this way[3][4], yet I
can confirm FreeBSD still has the problem. (DragonflyBSD has nearly
identical code.) But that implementation duplicates the short-circuit, along
with the bug of not accounting for skip1 and skip2, in cmp.c as part of
implementing the -z flag[5]:

        if (special)
                c_special(fd1, file1, skip1, fd2, file2, skip2);
        else {
                if (zflag && sb1.st_size != sb2.st_size) {
                        if (!sflag)
                                (void) printf("%s %s differ: size\n",
                                    file1, file2);
                        exit(DIFF_EXIT);
                }
                c_regular(fd1, file1, skip1, sb1.st_size,
                    fd2, file2, skip2, sb2.st_size);
        }
        exit(0);

It appears that the June 20, 2000 fix to the short-circuit in regular.c
wasn't recognized during the July 14, 2000 -z feature addition.[6][7]

[1] https://cvsweb.openbsd.org/src/usr.bin/cmp/regular.c?rev=1.12
[2] https://cvsweb.openbsd.org/src/usr.bin/cmp/misc.c?rev=1.7
[3] 
https://gitweb.dragonflybsd.org/dragonfly.git/blob/4d4f84f:/usr.bin/cmp/regular.c
[4] https://svnweb.freebsd.org/base/head/usr.bin/cmp/regular.c?revision=344551
[5] 
https://svnweb.freebsd.org/base/head/usr.bin/cmp/cmp.c?revision=344551&view=markup#l193
[6] 
https://svnweb.freebsd.org/base/head/usr.bin/cmp/regular.c?revision=61883&view=markup
[7] 
https://svnweb.freebsd.org/base/head/usr.bin/cmp/cmp.c?view=markup&pathrev=63157

--- regular.c   6 Feb 2015 23:21:59 -0000       1.12
+++ regular.c   9 Jan 2021 07:51:13 -0000
@@ -51,15 +51,15 @@ c_regular(int fd1, char *file1, off_t sk
        off_t byte, length, line;
        int dfound;
 
-       if (sflag && len1 != len2)
-               exit(1);
-
        if (skip1 > len1)
                eofmsg(file1);
        len1 -= skip1;
        if (skip2 > len2)
                eofmsg(file2);
        len2 -= skip2;
+
+       if (sflag && len1 != len2)
+               exit(1);
 
        length = MINIMUM(len1, len2);
        if (length > SIZE_MAX) {

Reply via email to