[PATCH] Include header required for INT_MAX

[PATCH] Include header required for INT_MAX

[Thomas Koeller]

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

Signed-off-by: Thomas Koeller <thomas@koeller.dyndns.org>

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

Re: [PATCH] Include header required for INT_MAX

[Christoph Hellwig]

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.

Re: [PATCH] Include header required for INT_MAX

[Thomas Koeller]

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

Re: [PATCH] Include header required for INT_MAX

[Alexander E. Patrakov]

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

Re: [PATCH] Include header required for INT_MAX

[Jan Engelhardt]

>> 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.

Re: [PATCH] Include header required for INT_MAX

[Peter Zijlstra]

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?

Re: [PATCH] Include header required for INT_MAX

[Andreas Schwab]

Jan Engelhardt <jengelh@computergmbh.de> 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, schwab@suse.de
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."

Re: [PATCH] Include header required for INT_MAX

[Robert P. J. Day]

On Mon, 12 Nov 2007, Andreas Schwab wrote:

> Jan Engelhardt <jengelh@computergmbh.de> 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
========================================================================

Re: [PATCH] Include header required for INT_MAX

[Andreas Schwab]

"Robert P. J. Day" <rpjday@crashcourse.ca> writes:

> On Mon, 12 Nov 2007, Andreas Schwab wrote:
>
>> Jan Engelhardt <jengelh@computergmbh.de> 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, schwab@suse.de
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."

Re: [PATCH] Include header required for INT_MAX

[Vegard Nossum]

On Nov 12, 2007 1:06 PM, Andreas Schwab <schwab@suse.de> wrote:
> "Robert P. J. Day" <rpjday@crashcourse.ca> writes:
>
> > On Mon, 12 Nov 2007, Andreas Schwab wrote:
> >
> >> Jan Engelhardt <jengelh@computergmbh.de> 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

Re: [PATCH] Include header required for INT_MAX

[Jan Engelhardt]

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.

Re: [PATCH] Include header required for INT_MAX

[Mike Frysinger]

On Nov 11, 2007 5:52 PM, Jan Engelhardt <jengelh@computergmbh.de> 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