On Fri, Nov 20, 2015 at 9:06 PM, Stephan Müller <frukto...@gmail.com> wrote:
> Am Fri, 20 Nov 2015 18:28:37 +0100
> schrieb Jim Meyering <j...@meyering.net>:
>
>> On Tue, Nov 17, 2015 at 12:44 PM, Stephan Müller
>> <frukto...@gmail.com> wrote:
>> > recently I had to debug weird problem. Finally I figured it out.
>> >
>> > Virtual file systems like /sys or /proc usually don't care about
>> > file sizes. All files have a size of 0. This leads to difficulties
>> > as diff sometimes looks for file sizes.
>> >
>> > Say you do:
>> >
>> >> $ cp /proc/cmdline my_cmdline
>> >> $ diff /proc/cmdline my_cmdline ; echo $?
>> >> 0 // ok, files don't differ
>> >> $ diff --brief /proc/cmdline my_cmdline
>> >> Files /proc/cmdline and mycmdline differ
>> >
>> > The --brief option triggers a binary compare, as we aren't
>> > interested in the actual differences this makes sense. As a first
>> > step, file sizes are compared (0 vs ~150) and the files are
>> > reported as different.
>>
>> thanks for the report.
>> What version of diffutils are you using?
>> I think this has been fixed for some time.
>> I was unable to reproduce with 2.8.1 nor with the latest built from
>> git. I.e., I created an empty file and used diff-2.8.1 to compare it
>> with the nominally-
>> zero-length /proc/cmdline file, and diff did the right thing.
>> Also, I ran stat to show st_size of each file is indeed 0:
>>
>> $ : > /tmp/k; /p/p/diffutils-2.8.1/bin/diff /proc/cmdline /tmp/k; \
>> stat --format %s /proc/cmdline /tmp/k
>> 1d0
>> < ro root=LABEL=...
>> 0
>> 0
>>
>> In fact, I went ahead and built all available versions and tested them
>> like this:
>>
>> $ for i in /p/p/*/bin/diff; do p=diffutils-$i; echo $i; $i
>> /proc/cmdline /tmp/k > /dev/null && echo bad; done
>> /p/p/diffutils-2.7/bin/diff
>> /p/p/diffutils-2.8.1/bin/diff
>> /p/p/diffutils-2.8/bin/diff
>> /p/p/diffutils-2.9/bin/diff
>> /p/p/diffutils-3.0/bin/diff
>> /p/p/diffutils-3.1/bin/diff
>> /p/p/diffutils-3.2/bin/diff
>> /p/p/diffutils-3.3/bin/diff
>
> Hi,
>
> I am using v.3.3 of diffutils
>
> $ diff -v
> diff (GNU diffutils) 3.3
>
> but I think you misunderstood the problem. Sorry for being ambiguous. I
> am not diffing against an empty file. That works well. The point is
> procfs doesn't care about size, but 'normal' file systems do. So for
> example on my system I have (after cp /proc/cmdline mycmdline)
>
> $ stat --format %s /proc/cmdline mycmdline
> 0
> 140
>
> The result of diffing /proc/cmdline against mycmdline depends on the
> --brief flag.
>
> STEPS TO REPRODUCE:
>
> cp /proc/cmdline mycmdline
> diff --brief /proc/cmdline mycmdline > /dev/null ; echo ?$
> 1
> diff /proc/cmdline mycmdline ; echo $?
> 0
>
> EXPECTED RESULT:
>
> cp /proc/cmdline mycmdline
> diff --brief /proc/cmdline mycmdline > /dev/null ; echo ?$
> 0
> diff /proc/cmdline mycmdline ; echo $?
> 0
Oh, indeed. Thank you for clarifying. That feels like a bug.
Here's a knee-jerk patch that refrains from using the st_size-comparing
heuristic when either of the sizes is zero. This may well be wrong.
I have only barely tested the diff.c code path.
diff --git a/src/analyze.c b/src/analyze.c
index 893d07c..2622810 100644
--- a/src/analyze.c
+++ b/src/analyze.c
@@ -477,6 +477,8 @@ diff_2_files (struct comparison *cmp)
{
/* Files with different lengths must be different. */
if (cmp->file[0].stat.st_size != cmp->file[1].stat.st_size
+ && 0 < cmp->file[0].stat.st_size
+ && 0 < cmp->file[1].stat.st_size
&& (cmp->file[0].desc < 0 || S_ISREG (cmp->file[0].stat.st_mode))
&& (cmp->file[1].desc < 0 || S_ISREG (cmp->file[1].stat.st_mode)))
changes = 1;
diff --git a/src/diff.c b/src/diff.c
index efd7e47..b2a90aa 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -1344,7 +1344,9 @@ compare_files (struct comparison const *parent,
else if (files_can_be_treated_as_binary
&& S_ISREG (cmp.file[0].stat.st_mode)
&& S_ISREG (cmp.file[1].stat.st_mode)
- && cmp.file[0].stat.st_size != cmp.file[1].stat.st_size)
+ && cmp.file[0].stat.st_size != cmp.file[1].stat.st_size
+ && 0 < cmp.file[0].stat.st_size
+ && 0 < cmp.file[1].stat.st_size)
{
message ("Files %s and %s differ\n",
file_label[0] ? file_label[0] : cmp.file[0].name,
