Re: [PATCH] r1986 broke FAT detection

[Javier Martín]

El mar, 10-02-2009 a las 10:50 +0100, Felix Zielcke escribió:
> Am Dienstag, den 10.02.2009, 01:19 +0100 schrieb Javier Martín:
> > At r1985, "sudo ./grub-probe -t fs -d /dev/fd0" outputs "fat" with a
> > freshly-formatted VFAT floppy in the drive. At r1986, it spits "error:
> > unknown filesystem". The cause is this error, repeated three times:
> > 
> > if (! grub_strncmp(something, "FAT12", 5))
> >   goto fail;
> > 
> > Strncmp does not return a boolean result (i.e. matches or doesn't), but
> > an _integer_ that is supposed to establish a comparison order between
> > strings. Thus, a return value of 0 is actually a match. See why I insist
> > on treating semantic-ints different than semantic-bools even though the
> > language does not? The correction is obvious (a patch is attached):
> 
> Thanks for your patch. Commited.
> 
You're welcome. I see that nevertheless the "0 != " comparisons were
substituted for standard C int-to-bool-conversion-based comparisons.
Maybe people should know the signature _and_ semantic contract of
strncmp, but frequently they don't (I had to look it up in the
handbook), and while the code that was committed may look like an
"obvious error" to a wanderer (because, of course, comparison functions
should return a semantic-bool, shouldn't they?), the version with the
explicit "0 != " checks at least looks like it was written like that _on
purpose_ (and the actual binary cost should be zero with any sensible
compiler), thus making future developers on bug-fixing quests at least
scratch their heads before proposing the change to the "if (!strncmp)"
error. So, keeping the coding style consistent is important, but I think
a balance with readability is in order. Thus, you are the maintainers
and you know what you're doing, but I think it's not worth to keep the
coding style so strict as to become confusing.


-- Lazy Oblivious, Rational Disaster -- Habbit

signature.asc
Description: Esta parte del mensaje está firmada digitalmente