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
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
