This is the mail archive of the glibc-bugs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug libc/14362] Incorrect ioctl declaration


http://sourceware.org/bugzilla/show_bug.cgi?id=14362

--- Comment #5 from Linus Torvalds <torvalds@linux-foundation.org> 2012-07-24 17:41:28 UTC ---
(In reply to comment #4)
> Link to man-pages bugzilla.
> 
>    https://bugzilla.kernel.org/show_bug.cgi?id=42705

I think that one brings up interesting points, but is not actually correct or
really relevant. But it does show some of the confusion in this area, yes,
which makes it interesting.

The Linux kernel internally uses 'unsigned int' exactly because sign extension
is such a ^&%*@ confusing pain in the ass when you have random big integers
(and yes, they really historically are big, with high bits used for various
things).

At the same time, in Linux, the kernel doesn't even really care: the sign
extension is irrelevant as long as the comparison is always done in 32 bits. So
the important part is not the "unsigned int", but that it is *some* kind of
"int". Not a "long".

But I point out the internal use of "unsigned int", because I do think it's
nice: it avoids the worry about sign extension, and makes things that have the
high bit set not have any special behavior. So "unsigned int" is a better type,
I think.

But when that bugzilla entry says that

        int x = IOCTL;
        if (x == IOCTL)
                /* ... */

  Will generate a "comparison always false due to limited range of data type"
  warning from gcc, and presumably will indeed compare false, which is clearly
  not the expected result.

it is actually incorrect. Because the IOCTL value is (or rather - can be) an
'unsigned int', when you do the comparison, the C typecasting rules will do the
comparison in 'unsigned int', and everything will be correct. The manpages
bugzilla entry is correct about this being *confusing*, though, and might
generate warnings about comparing signed and unsigned values.

Now, if you do things in "long", you really can get screwed. Then you might
sign-extend one and compare it against a non-sign-extended one, and then you're
really screwed. That is, in fact, exactly what happens on OS X right now due to
the OS X *incorrect* use of "unsigned long".

So I think the manpages bugzilla is wrong, but does bring up a valid source of
confusion. I absolutely agree that we would have been *much* better off if
POSIX had just said that the argument is "unsigned int".

But that's not what they did. And I think we should conform to POSIX, and admit
that it's ugly, but that it's absolutely better than "unsigned long", which is
just broken and causes actual bugs.

(Side note: the bugzilla would be correct if some IOCTL value is declared as a
single constant, eg

   #define IOCTL 2147483648

because such a constant would be implicitly converted to "long", and then the
comparison with an "int" (which would be sign-extended) would never be true.

However, that's not how the ioctl's with the high bits set are defined, they
are done by shifting values that are of type "unsigned int" around, or done as
hex constants, both of which will stay as "unsigned int"). And mixing "unsigned
int" and "int" values can be *confusing*, but it won't be wrong - nothing will
be extended to "long", but "int" will be silently converted to "unsigned int".

Yeah, C type conversion can be subtle.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]