2007-11-10 15:17:20

by Thomas Koeller

[permalink] [raw]
Subject: [PATCH] Include header required for INT_MAX

cdrom.h uses INT_MAX, so it must include kernel.h or
limits.h (userspace) for a definition.

Signed-off-by: Thomas Koeller <[email protected]>

diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index c6d3e22..bd8064a 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -12,6 +12,11 @@
#define _LINUX_CDROM_H

#include <asm/byteorder.h>
+#ifdef __KERNEL__
+#include <linux/kernel.h>
+#else
+#include <limits.h>
+#endif

/*******************************************************
* As of Linux 2.1.x, all Linux CD-ROM application programs will use this
--
1.5.3.4


2007-11-10 15:56:24

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

On Sat, Nov 10, 2007 at 03:55:15PM +0100, Thomas Koeller wrote:
> cdrom.h uses INT_MAX, so it must include kernel.h or
> limits.h (userspace) for a definition.

Nack, we shoiuld never include userspace headers in kernel headers,
an even more never add !__KERNEL__ ifdefs. Just make sure your
programs include limit.h before including linux/cdrom.h.

2007-11-10 16:50:24

by Alexander E. Patrakov

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

Thomas Koeller wrote:
> cdrom.h uses INT_MAX, so it must include kernel.h or
> limits.h (userspace) for a definition.

No, it must be fixed so that it doesn't use this #defined constant. Debian has this:

/* Special codes used when specifying changer slots. */
#define CDSL_NONE ((int) (~0U>>1)-1)
#define CDSL_CURRENT ((int) (~0U>>1))

--
Alexander E. Patrakov

2007-11-11 00:52:56

by Thomas Koeller

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

On Samstag, 10. November 2007, Christoph Hellwig wrote:
> On Sat, Nov 10, 2007 at 03:55:15PM +0100, Thomas Koeller wrote:
> > cdrom.h uses INT_MAX, so it must include kernel.h or
> > limits.h (userspace) for a definition.
>
> Nack, we shoiuld never include userspace headers in kernel headers,
> an even more never add !__KERNEL__ ifdefs. Just make sure your
> programs include limit.h before including linux/cdrom.h.

I think header files should be complete, and should not use undefined
macros, picking up every random definition that may be in effect when
the header is included, don't you agree? Why not use some other
constant instead of INT_MAX? What improvement does commit
132e4b0a049c39337c535501561b8301c7f2b202 provide? It certainly
breaks existing userspace code.

tk

--
Thomas Koeller
thomas at koeller dot dyndns dot org

2007-11-11 22:52:34

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX


>> Nack, we shoiuld never include userspace headers in kernel headers,
>> an even more never add !__KERNEL__ ifdefs. Just make sure your
>> programs include limit.h before including linux/cdrom.h.
>
>I think header files should be complete, and should not use undefined
>macros, picking up every random definition that may be in effect when
>the header is included, don't you agree?

No, because I be damn sure that some developers try compiling programs
in non-linux environments (cygwin, solaris, andyourpersonaldistro, you
name it) which do not have to adhere to <limits.h>. It might use
<cosmiclimits.h> instead, or whatever.
Hence, such extra includes are a nogo.

2007-11-12 10:21:21

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX


On Sun, 2007-11-11 at 23:52 +0100, Jan Engelhardt wrote:
> >> Nack, we shoiuld never include userspace headers in kernel headers,
> >> an even more never add !__KERNEL__ ifdefs. Just make sure your
> >> programs include limit.h before including linux/cdrom.h.
> >
> >I think header files should be complete, and should not use undefined
> >macros, picking up every random definition that may be in effect when
> >the header is included, don't you agree?
>
> No, because I be damn sure that some developers try compiling programs
> in non-linux environments (cygwin, solaris, andyourpersonaldistro, you
> name it) which do not have to adhere to <limits.h>. It might use
> <cosmiclimits.h> instead, or whatever.
> Hence, such extra includes are a nogo.

Surely ISO-C99 has something to say on this subject?

2007-11-12 10:46:53

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

Jan Engelhardt <[email protected]> writes:

> No, because I be damn sure that some developers try compiling programs
> in non-linux environments (cygwin, solaris, andyourpersonaldistro, you
> name it) which do not have to adhere to <limits.h>. It might use
> <cosmiclimits.h> instead, or whatever.

Every C compiler has <limit.h>.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2007-11-12 10:55:11

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

On Mon, 12 Nov 2007, Andreas Schwab wrote:

> Jan Engelhardt <[email protected]> writes:
>
> > No, because I be damn sure that some developers try compiling programs
> > in non-linux environments (cygwin, solaris, andyourpersonaldistro, you
> > name it) which do not have to adhere to <limits.h>. It might use
> > <cosmiclimits.h> instead, or whatever.
>
> Every C compiler has <limit.h>.

i'm assuming you mean <limits.h>, no?

rday
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

2007-11-12 12:07:03

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

"Robert P. J. Day" <[email protected]> writes:

> On Mon, 12 Nov 2007, Andreas Schwab wrote:
>
>> Jan Engelhardt <[email protected]> writes:
>>
>> > No, because I be damn sure that some developers try compiling programs
>> > in non-linux environments (cygwin, solaris, andyourpersonaldistro, you
>> > name it) which do not have to adhere to <limits.h>. It might use
>> > <cosmiclimits.h> instead, or whatever.
>>
>> Every C compiler has <limit.h>.
>
> i'm assuming you mean <limits.h>, no?

Yes, sorry for the typo.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2007-11-12 12:57:19

by Vegard Nossum

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

On Nov 12, 2007 1:06 PM, Andreas Schwab <[email protected]> wrote:
> "Robert P. J. Day" <[email protected]> writes:
>
> > On Mon, 12 Nov 2007, Andreas Schwab wrote:
> >
> >> Jan Engelhardt <[email protected]> writes:
> >>
> >> > No, because I be damn sure that some developers try compiling programs
> >> > in non-linux environments (cygwin, solaris, andyourpersonaldistro, you
> >> > name it) which do not have to adhere to <limits.h>. It might use
> >> > <cosmiclimits.h> instead, or whatever.
> >>
> >> Every C compiler has <limit.h>.
> >
> > i'm assuming you mean <limits.h>, no?
>
> Yes, sorry for the typo.

This seems like a good time to ask why the kernel doesn't use
<stdint.h> for its INT_MAX and type definitions like uint32_t., etc.
>From the manpage: "The <stdint.h> header is a subset of the
<inttypes.h> header more suitable for use in freestanding
environments, which might not support the formatted I/O functions. In
some environments, if the formatted conversion support is not
wanted, using this header instead of the <inttypes.h> header avoids
defining such a large number of macros."

limits.h, on the other hand, seems to define a lot of
userspace-related things like ARG_MAX, ATEXIT_MAX. Or BC_BASE_MAX
(Maximum obase values allowed by the bc utility.). Which have nothing
to do with the kernel at all.

So even though another header is mentioned (inttypes.h), the argument
in favour of using stdint.h still holds; it defines the subset of
macros/types that is suitable in a freestanding environment (i.e. the
kernel).

Vegard

2007-11-12 13:38:06

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX


On Nov 12 2007 13:57, Vegard Nossum wrote:
>
>This seems like a good time to ask why the kernel doesn't use
><stdint.h> for its INT_MAX and type definitions like uint32_t., etc.
>>From the manpage: "The <stdint.h> header is a subset of the
><inttypes.h> header more suitable for use in freestanding
>environments, which might not support the formatted I/O functions. In
>some environments, if the formatted conversion support is not
>wanted, using this header instead of the <inttypes.h> header avoids
>defining such a large number of macros."

Yes, I am all for that, replacing u8 __u8 with uint8_t, and so on.

2007-11-12 20:51:12

by Mike Frysinger

[permalink] [raw]
Subject: Re: [PATCH] Include header required for INT_MAX

On Nov 11, 2007 5:52 PM, Jan Engelhardt <[email protected]> wrote:
> >> Nack, we shoiuld never include userspace headers in kernel headers,
> >> an even more never add !__KERNEL__ ifdefs. Just make sure your
> >> programs include limit.h before including linux/cdrom.h.
> >
> >I think header files should be complete, and should not use undefined
> >macros, picking up every random definition that may be in effect when
> >the header is included, don't you agree?
>
> No, because I be damn sure that some developers try compiling programs
> in non-linux environments (cygwin, solaris, andyourpersonaldistro, you
> name it) which do not have to adhere to <limits.h>. It might use
> <cosmiclimits.h> instead, or whatever.
> Hence, such extra includes are a nogo.

why are non-linux environments even relevant to the discussion ?
we're talking about fixing up a *linux header* file, so anyone doing
"#include <linux/cdrom.h>" in a non-linux environment is just stupid.

if yourpersonaldistro is a pos and doesnt provide a POSIX compliant
limits.h, then that distro is garbage and should be deleted and have
no bearing whatsoever on the direction of linux.
-mike