Download raw body.
diff(1) -s option can produce erroneous positives
When diff(1) is used with the -s option, files can be explicitly reported as
being identical despite actually being different, in two distinct situations:
* Filesystem permissions prevent the opening of one or both files.
Or
* One or both of the files is not a regular file, (E.G. either is a socket).
Reproducers:
$ echo foo > file_1
$ echo bar > file_2
$ chmod 000 file_1 file_2
$ diff -s file_1 file_2
diff: file_1: Permission denied
Files file_1 and file_2 are identical
$ echo foo > file_3
$ diff -s file_3 /dev/log
diff: /dev/log: Operation not supported
Files file_3 and /dev/log are identical
Note that whilst the 'permission denied' message goes to stderr, the 'files
are identical' message is output to stdout. Any usage, (in scripts, etc),
that redirects stderr elsewhere makes it entirely plausible that in practical
usage, the 'identical' message could be seen without the permissions warning.
The proposed fix below introduces D_INDETERMINATE to handle such cases by
suppressing the erroneous output.
Caveat: if _both_ files are unreadable, the current behaviour of -s is to
report identical regardless:
$ diff -s /dev/kmem /dev/kmem
diff: /dev/kmem: Permission denied
Files /dev/kmem and /dev/kmem are identical
$ echo foo > file_4
$ chmod 000 file_4
$ diff -s file_4 file_4
diff: file_4: Permission denied
Files file_4 and file_4 are identical
By pure chance, this handles the special case of a file being compared against
itself correctly, (assuming that the underlying filesystem guarantees that the
same file is accessed both times).
The proposed fix changes this behaviour (*), as it prevents 'identical' being
returned when either or both files cannot be opened.
Although diff(1) does check the supplied files' inode numbers for equality,
the code to do this is called afterwards in files_differ(), and not reached if
either or both files cannot be opened.
(*) Note that this only affects the printed output from -s, the exit status of
diff(1) will be non-zero both with and without the proposed patch:
$ diff -s file_4 file_4
diff: file_4: Permission denied
Files file_4 and file_4 are identical
$ echo $?
2
So the proposed patch is not expected to break scripts which rely on the exit
status alone.
--- usr.bin/diff/diff.h
+++ usr.bin/diff/diff.h
@@ -75,6 +75,7 @@
#define D_MISMATCH2 4 /* path1 was a file, path2 a dir */
#define D_SKIPPED1 5 /* path1 was a special file */
#define D_SKIPPED2 6 /* path2 was a special file */
+#define D_INDETERMINATE 7 /* One or both files could not be opened */
struct excludes {
char *pattern;
--- usr.bin/diff/diffreg.c
+++ usr.bin/diff/diffreg.c
@@ -318,6 +318,7 @@
fstat(fileno(f1), &stb1) == -1) {
warn("%s", file1);
status |= 2;
+ rval = D_INDETERMINATE;
goto closem;
}
} else if (strcmp(file1, "-") == 0)
@@ -328,6 +329,7 @@
if (f1 == NULL) {
warn("%s", file1);
status |= 2;
+ rval = D_INDETERMINATE;
goto closem;
}
@@ -339,6 +341,7 @@
fstat(fileno(f2), &stb2) == -1) {
warn("%s", file2);
status |= 2;
+ rval = D_INDETERMINATE;
goto closem;
}
} else if (strcmp(file2, "-") == 0)
@@ -349,6 +352,7 @@
if (f2 == NULL) {
warn("%s", file2);
status |= 2;
+ rval = D_INDETERMINATE;
goto closem;
}
diff(1) -s option can produce erroneous positives