Re: [PATCH] r1986 broke FAT detection

[Javier Martín]

El sáb, 21-02-2009 a las 14:13 +0100, Robert Millan escribió:
> On Tue, Feb 10, 2009 at 12:44:14PM +0100, Javier Martín wrote:
> > > 
> > 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.
> 
> I think you're confusing things.  C has no boolean type.  I know strcmp
> gives more info than just a semantic boolean, but in this case it's not
> interesting to us.
> 
My point is that the current code _looks_ confusing: due to the lack of
a proper boolean type in C (and no, C99 _Bool does not count either),
functions that return "semantic-booleans" return int's instead, with the
C convention of 0->false, other->true. This is a common convention, and
there are heaps of functions that work like that. So many, in fact, that
those invocations of strncmp are likely to look odd to someone that does
not have the semantic contract (and not just the formal C signature) in
mind; because under the "usual" convention the code _seems_ to be
checking whether the filesystem is fat12, fat16 _and_ fat32 at once.
However, the return value of strncmp is actually a semantic integer, and
thus the Right Thing (TM, and sorry if I sound preaching) is to perform
the very exact comparison we want, that is "strncmp() == 0".

As I already said, the explicit integer comparison would, given its
relative rarity in normal C code, at least make people scratch their
heads before thinking "hey, this comparison should use 'or's instead of
'and's": in fact, I think that's the advantage of my version - it keeps
the original "x or y or z" logical structure of the comparison. Given
that the cost in binary size would most likely be zero for any average
compiler, I think the change is worth performing.

-- 
-- Lazy, Oblivious, Rational Disaster -- Habbit

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