2003-09-25 09:46:45

by Ronald S. Bultje

[permalink] [raw]
Subject: linux/time.h annoyance

Hi,

I'm annoyed by something in linux/time.h. The issue is as follows:

-
#include <sys/time.h>
#include <linux/time.h>
int main () { return 0; }
-

... compile with -Wall -Werror, and you'll get ...

-
In file included from test.c:2:
/usr/include/linux/time.h:9: redefinition of `struct timespec'
/usr/include/linux/time.h:88: redefinition of `struct timeval'
/usr/include/linux/time.h:93: redefinition of `struct timezone'
/usr/include/linux/time.h:124: redefinition of `struct itimerval'
-

... and the compile fails. In 2.6.x, there'll be some additional
warnings ...

-
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:9: redefinition of
`struct timespec'
/usr/src/linux-2.6.0-test4/include/linux/time.h:15: redefinition of
`struct timeval'
/usr/src/linux-2.6.0-test4/include/linux/time.h:20: redefinition of
`struct timezone'
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:341:1: "FD_SET"
redefined
In file included from /usr/include/sys/time.h:31,
from test.c:1:
/usr/include/sys/select.h:93:1: this is the location of the previous
definition
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:342:1: "FD_CLR"
redefined
In file included from /usr/include/sys/time.h:31,
from test.c:1:
/usr/include/sys/select.h:94:1: this is the location of the previous
definition
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:343:1: "FD_ISSET"
redefined
In file included from /usr/include/sys/time.h:31,
from test.c:1:
/usr/include/sys/select.h:95:1: this is the location of the previous
definition
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:344:1: "FD_ZERO"
redefined
In file included from /usr/include/sys/time.h:31,
from test.c:1:
/usr/include/sys/select.h:96:1: this is the location of the previous
definition
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:350:1: "ITIMER_REAL"
redefined
In file included from test.c:1:
/usr/include/sys/time.h:96:1: this is the location of the previous
definition
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:351:1: "ITIMER_VIRTUAL"
redefined
In file included from test.c:1:
/usr/include/sys/time.h:99:1: this is the location of the previous
definition
In file included from test.c:2:
/usr/src/linux-2.6.0-test4/include/linux/time.h:352:1: "ITIMER_PROF"
redefined
In file included from test.c:1:
/usr/include/sys/time.h:103:1: this is the location of the previous
definition
/usr/src/linux-2.6.0-test4/include/linux/time.h:359: redefinition of
`struct itimerval'
-

... and it still fails.

This is an issue in various ways. Most importantly, several linux kernel
headers that are not found in sys/ or for which sys/ is simply a
boilerplate-code-include (such as sys/soundcard.h) include linux/time.h
themselves (example: linux/videodev2.h). Also, many userspace tools
include sys/time.h automatically (example: glib2). Consequently, without
using ugly hacks, such as #define _LINUX_TIME_H 1 before including
glib.h and linux/videodev2.h, I cannot make a gnome2 video application
that will compile properly with -Wall -Werror. And that sucks.

sys/time.h & related headers seems to have a few macros to check for
struct definitions for each of the above-mentioned. I'm not sure how
official any of this is, but the fix for the first two of the warnings
seems very simple: add the same macro checks to linux/time.h. For struct
timespec, this is __timespec_defined. For struct timeval, this is
_STRUCT_TIMEVAL. Amusingly, linux/time.h uses _STRUCT_TIMESPEC for the
first. Code would look like:

#ifndef macro
#define macro
struct foo {
int bar;
}
#endif /* macro */

The third and fourth warning (timezone/itimerval) are harder, because
sys/time.h & co don't have macros to check for redifinitions here. Does
anyone have suggestions on how to fix those two? Perhaps a check to see
whether time.h or sys/time.h was already included before defining the
structs (although that would still break if you reverse the order of
including those files, i.e. first #include linux/time.h and then
sys/time.h)?
For the 2.6.x-only warnings (FD_CLR etc.), these are all macro's and
could thus simply be checked by doing a "#ifndef ... #define ... #endif
/* ... */" for each of the ones that give a warning.

If wanted, I can provide a patch to fix parts of this, but this is so
simple that it shouldn't be needed for me to send one. Besides, I'm not
sure whether my proposed fixes are the right ones.

Thanks,

Ronald
(please CC me, I'm not subscribed)

--
Ronald Bultje <[email protected]>


2003-09-25 09:54:53

by Christoph Hellwig

[permalink] [raw]
Subject: Re: linux/time.h annoyance

On Thu, Sep 25, 2003 at 11:48:01AM +0200, Ronald Bultje wrote:
> Hi,
>
> I'm annoyed by something in linux/time.h. The issue is as follows:
>
> -
> #include <sys/time.h>
> #include <linux/time.h>
> int main () { return 0; }

So don't include it. Userspace should use <sys/time.h>, not kernel
headers.

2003-09-25 10:15:53

by Ronald S. Bultje

[permalink] [raw]
Subject: Re: linux/time.h annoyance

On Thu, 2003-09-25 at 11:54, Christoph Hellwig wrote:
> On Thu, Sep 25, 2003 at 11:48:01AM +0200, Ronald Bultje wrote:
> > Hi,
> >
> > I'm annoyed by something in linux/time.h. The issue is as follows:
> >
> > -
> > #include <sys/time.h>
> > #include <linux/time.h>
> > int main () { return 0; }
>
> So don't include it. Userspace should use <sys/time.h>, not kernel
> headers.

linux/videodev2.h includes linux/time.h. And I need linux/videodev2.h in
my application, there is no sys/ equivalent. I expect there's more of
such cases.

I also explained this in my first email. ;).

Ronald

--
Ronald Bultje <[email protected]>
Linux Video/Multimedia developer

2003-09-25 10:23:30

by Christoph Hellwig

[permalink] [raw]
Subject: Re: linux/time.h annoyance

On Thu, Sep 25, 2003 at 12:17:12PM +0200, Ronald Bultje wrote:
> linux/videodev2.h includes linux/time.h. And I need linux/videodev2.h in
> my application, there is no sys/ equivalent. I expect there's more of
> such cases.
>
> I also explained this in my first email. ;).

So fix your copy of linux/videdev2.h to not include linux/time.h.

If you ask Gerd nicely he might even include that change in the kernel
version so don't have to keep a delta.

2003-09-25 10:54:16

by Ronald S. Bultje

[permalink] [raw]
Subject: Re: linux/time.h annoyance

Hi,

[CC to v4l/Gerd then]

On Thu, 2003-09-25 at 12:23, Christoph Hellwig wrote:
> On Thu, Sep 25, 2003 at 12:17:12PM +0200, Ronald Bultje wrote:
> > linux/videodev2.h includes linux/time.h. And I need linux/videodev2.h in
> > my application, there is no sys/ equivalent. I expect there's more of
> > such cases.
>
> So fix your copy of linux/videdev2.h to not include linux/time.h.
>
> If you ask Gerd nicely he might even include that change in the kernel
> version so don't have to keep a delta.

Hm, makes sense... Gerd, could you please remove linux/time.h from
linux/videodev2.h?

Thanks,
Ronald

--
Ronald Bultje <[email protected]>

2003-09-25 11:08:06

by MånsRullgård

[permalink] [raw]
Subject: Re: linux/time.h annoyance

Christoph Hellwig <[email protected]> writes:

>> linux/videodev2.h includes linux/time.h. And I need linux/videodev2.h in
>> my application, there is no sys/ equivalent. I expect there's more of
>> such cases.
>>
>> I also explained this in my first email. ;).
>
> So fix your copy of linux/videdev2.h to not include linux/time.h.
>
> If you ask Gerd nicely he might even include that change in the kernel
> version so don't have to keep a delta.

I've been wondering for some time if I was the only one to see these
types of errors. There are other headers having the same problems,
but I can't remember which ones right now.

--
M?ns Rullg?rd
[email protected]

2003-09-25 11:25:18

by Gerd Knorr

[permalink] [raw]
Subject: Re: linux/time.h annoyance

> > If you ask Gerd nicely he might even include that change in the kernel
> > version so don't have to keep a delta.
>
> Hm, makes sense... Gerd, could you please remove linux/time.h from
> linux/videodev2.h?

I can't remove it, it will break in kernel space then. But my latest
version has it #ifdef'ed already (patches at bytesex.org/patches, as
usual ...).

Gerd

--
You have a new virus in /var/mail/kraxel

2003-09-25 18:01:47

by Sam Ravnborg

[permalink] [raw]
Subject: Re: linux/time.h annoyance

On Thu, Sep 25, 2003 at 10:54:37AM +0100, Christoph Hellwig wrote:
> On Thu, Sep 25, 2003 at 11:48:01AM +0200, Ronald Bultje wrote:
> > Hi,
> >
> > I'm annoyed by something in linux/time.h. The issue is as follows:
> >
> > -
> > #include <sys/time.h>
> > #include <linux/time.h>
> > int main () { return 0; }
>
> So don't include it. Userspace should use <sys/time.h>, not kernel
> headers.

What about the patch that Matthew Wilcox posted?
Was it too late or was it not the right solution for separating
headers to be exposed to user space, and kernel internal headers?

Sam