Re: [alsa-devel] [alsa-cvslog] alsa-lib: Add struct timeval and timespec definition when _POSIX_C_SOURCE is not defined
On Fri, Oct 06, 2006 at 11:01:44AM +0200, Jaroslav Kysela wrote:
changeset: 2218:603411fd0c77712cab43cdd5afcf0ff4ebe51a7f tag: tip user: perex date: Fri Oct 06 11:01:40 2006 +0200 files: include/global.h description: Add struct timeval and timespec definition when _POSIX_C_SOURCE is not defined
FIXME: It might cause problems on some platforms when tv_usec is not long type.
And now it also breaks compilation of the following code (at least with glibc 2.5):
#include <sys/select.h> #include <alsa/global.h>
$ gcc -c --std=c99 x.c In file included from x.c:2: /usr/include/alsa/global.h:134: error: redefinition of 'struct timeval' /usr/include/alsa/global.h:139: error: redefinition of 'struct timespec'
(Without #include <sys/select.h> only 'struct timespec' is redefined.)
IMHO attempts to define structures which should be defined by system headers are too dangerous - if someone is using preprocessor flags which hide required parts of system headers, it is their problem, and the risk of mismatch is too high.
BTW, <time.h> does not provide 'struct timeval' at all, even with _GNU_SOURCE - it defines only 'struct timespec'; however, this does not cause problems, because ALSA headers currently use only pointers to snd_timestamp_t, which works even without a complete definition of 'struct timeval'. We can try to include <sys/time.h> to get 'struct timeval', or even <sys/select.h> (which currently seems to provide both 'struct timeval' and 'struct timespec' even with -ansi, and http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/select.h.html tells that it should always provide their definitions).
diff -r 7bc021fb4bf1027b23aa5429ceda1e1e35c7d126 -r 603411fd0c77712cab43cdd5afcf0ff4ebe51a7f include/global.h --- a/include/global.h Fri Oct 06 10:38:39 2006 +0200 +++ b/include/global.h Fri Oct 06 11:01:40 2006 +0200 @@ -130,6 +130,18 @@ int snd_shm_area_destroy(struct snd_shm_
int snd_user_file(const char *file, char **result);
+#ifndef _POSIC_C_SOURCE +struct timeval {
- time_t tv_sec; /* seconds */
- long tv_usec; /* microseconds */
+};
+struct timespec {
- time_t tv_sec; /* seconds */
- long tv_nsec; /* nanoseconds */
+}; +#endif
/** Timestamp */ typedef struct timeval snd_timestamp_t; /** Hi-res timestamp */
At Sun, 24 Jun 2007 17:35:25 +0400, Sergey Vlasov wrote:
On Fri, Oct 06, 2006 at 11:01:44AM +0200, Jaroslav Kysela wrote:
changeset: 2218:603411fd0c77712cab43cdd5afcf0ff4ebe51a7f tag: tip user: perex date: Fri Oct 06 11:01:40 2006 +0200 files: include/global.h description: Add struct timeval and timespec definition when _POSIX_C_SOURCE is not defined
FIXME: It might cause problems on some platforms when tv_usec is not long type.
And now it also breaks compilation of the following code (at least with glibc 2.5):
#include <sys/select.h> #include <alsa/global.h>
$ gcc -c --std=c99 x.c In file included from x.c:2: /usr/include/alsa/global.h:134: error: redefinition of 'struct timeval' /usr/include/alsa/global.h:139: error: redefinition of 'struct timespec'
(Without #include <sys/select.h> only 'struct timespec' is redefined.)
IMHO attempts to define structures which should be defined by system headers are too dangerous - if someone is using preprocessor flags which hide required parts of system headers, it is their problem, and the risk of mismatch is too high.
I agree.
Jaroslav, which system has no definition of these types? Maybe it'd be better to check it in configure script, rather than the simple ifdef.
BTW, <time.h> does not provide 'struct timeval' at all, even with _GNU_SOURCE - it defines only 'struct timespec'; however, this does not cause problems, because ALSA headers currently use only pointers to snd_timestamp_t, which works even without a complete definition of 'struct timeval'. We can try to include <sys/time.h> to get 'struct timeval', or even <sys/select.h> (which currently seems to provide both 'struct timeval' and 'struct timespec' even with -ansi, and http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/select.h.html tells that it should always provide their definitions).
A patch is welcome :)
thanks,
Takashi
On Mon, Jun 25, 2007 at 03:15:39PM +0200, Takashi Iwai wrote: [...]
#include <sys/select.h> #include <alsa/global.h>
$ gcc -c --std=c99 x.c In file included from x.c:2: /usr/include/alsa/global.h:134: error: redefinition of 'struct timeval' /usr/include/alsa/global.h:139: error: redefinition of 'struct timespec'
(Without #include <sys/select.h> only 'struct timespec' is redefined.)
IMHO attempts to define structures which should be defined by system headers are too dangerous - if someone is using preprocessor flags which hide required parts of system headers, it is their problem, and the risk of mismatch is too high.
I agree.
Jaroslav, which system has no definition of these types? Maybe it'd be better to check it in configure script, rather than the simple ifdef.
This won't help here - the above problem happens when alsa-lib headers are used by other programs, not when alsa-lib is built itself.
BTW, <time.h> does not provide 'struct timeval' at all, even with _GNU_SOURCE - it defines only 'struct timespec'; however, this does not cause problems, because ALSA headers currently use only pointers to snd_timestamp_t, which works even without a complete definition of 'struct timeval'. We can try to include <sys/time.h> to get 'struct timeval', or even <sys/select.h> (which currently seems to provide both 'struct timeval' and 'struct timespec' even with -ansi, and http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/select.h.html tells that it should always provide their definitions).
A patch is welcome :)
Just not sure which one...
The situation on my system (with glibc-2.5) is as follows:
- <time.h> defines 'struct timespec', unless -ansi (or -std=c99) option is used; it never defines 'struct timeval', even with -D_GNU_SOURCE (which should enable everything). This means that if only <time.h> is used, alsa/timer.h will not compile with -ansi due to the 'snd_htimestamp_t tstamp' member in snd_timer_tread_t, unless the program includes some other header which defines 'struct timespec' before alsa/timer.h.
- <sys/time.h> defines both 'struct timeval' and 'struct timespec', even with -ansi; however, according to the current Open Group spec (http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/time.h.html), <sys/time.h> must provide only 'struct timeval' and is not required to provide 'struct timespec', so the behavior may change in a future glibc release - again, a missing 'struct timespec' definition is fatal for alsa/timer.h.
- <sys/select.h> defines both 'struct timeval' and 'struct timespec' even with -ansi. In the Open Group spec <sys/select.h> is specified to define 'struct timeval', and also define 'struct timespec' "as described in <time.h>" (where it is described as optional), so it should be not worse than <time.h>.
Would it be OK to include <sys/select.h> instead of <time.h> and remove attempt to define system structs from alsa/global.h? Another option is to include both <time.h> and <sys/time.h>.
At Mon, 25 Jun 2007 19:24:56 +0400, Sergey Vlasov wrote:
On Mon, Jun 25, 2007 at 03:15:39PM +0200, Takashi Iwai wrote: [...]
#include <sys/select.h> #include <alsa/global.h>
$ gcc -c --std=c99 x.c In file included from x.c:2: /usr/include/alsa/global.h:134: error: redefinition of 'struct timeval' /usr/include/alsa/global.h:139: error: redefinition of 'struct timespec'
(Without #include <sys/select.h> only 'struct timespec' is redefined.)
IMHO attempts to define structures which should be defined by system headers are too dangerous - if someone is using preprocessor flags which hide required parts of system headers, it is their problem, and the risk of mismatch is too high.
I agree.
Jaroslav, which system has no definition of these types? Maybe it'd be better to check it in configure script, rather than the simple ifdef.
This won't help here - the above problem happens when alsa-lib headers are used by other programs, not when alsa-lib is built itself.
But, it can check at least whether the system-wide struct timeval / timespec definitions. It wasn't clear to me for which system these definitions were needed...
BTW, <time.h> does not provide 'struct timeval' at all, even with _GNU_SOURCE - it defines only 'struct timespec'; however, this does not cause problems, because ALSA headers currently use only pointers to snd_timestamp_t, which works even without a complete definition of 'struct timeval'. We can try to include <sys/time.h> to get 'struct timeval', or even <sys/select.h> (which currently seems to provide both 'struct timeval' and 'struct timespec' even with -ansi, and http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/select.h.html tells that it should always provide their definitions).
A patch is welcome :)
Just not sure which one...
The situation on my system (with glibc-2.5) is as follows:
<time.h> defines 'struct timespec', unless -ansi (or -std=c99) option is used; it never defines 'struct timeval', even with -D_GNU_SOURCE (which should enable everything). This means that if only <time.h> is used, alsa/timer.h will not compile with -ansi due to the 'snd_htimestamp_t tstamp' member in snd_timer_tread_t, unless the program includes some other header which defines 'struct timespec' before alsa/timer.h.
<sys/time.h> defines both 'struct timeval' and 'struct timespec', even with -ansi; however, according to the current Open Group spec (http://www.opengroup.org/onlinepubs/009695399/basedefs/sys/time.h.html), <sys/time.h> must provide only 'struct timeval' and is not required to provide 'struct timespec', so the behavior may change in a future glibc release - again, a missing 'struct timespec' definition is fatal for alsa/timer.h.
<sys/select.h> defines both 'struct timeval' and 'struct timespec' even with -ansi. In the Open Group spec <sys/select.h> is specified to define 'struct timeval', and also define 'struct timespec' "as described in <time.h>" (where it is described as optional), so it should be not worse than <time.h>.
Would it be OK to include <sys/select.h> instead of <time.h> and remove attempt to define system structs from alsa/global.h? Another option is to include both <time.h> and <sys/time.h>.
Yes, sys/select.h sounds reasonable.
thanks,
Takashi
participants (2)
-
Sergey Vlasov -
Takashi Iwai