Speex dev - Jul 2015 - [PATCH speexdsp] Don't rely on HAVE_STDINT_H et al. being defined

From: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>

Not everyone who includes speexdsp_config_types.h will have a test
which defines those, and if we've chosen to use the stdint types at
configure time then we know exactly which header(s) are available, so
just choose the best one then and generate the header to use it.

This patch, including the above text, is copied from a commit in the
speex repository[1]. The original commit for speex was made by Ron
<ron at debian.org>.

[1]
https://git.xiph.org/?p=speex.git;a=commitdiff;h=774c87d6cb7dd8dabdd17677fc6da753ecf4aa87

Signed-off-by: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
---
 configure.ac                             | 6 ++++++
 include/speex/speexdsp_config_types.h.in | 8 +-------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2cd2d1e..1de0c23 100644
--- a/configure.ac
+++ b/configure.ac
@@ -334,6 +334,12 @@ AC_SUBST([USIZE16])
 AC_SUBST([SIZE32])
 AC_SUBST([USIZE32])
 
+AS_IF([test "$ac_cv_header_stdint_h" = "yes"],   
[INCLUDE_STDINT="#include <stdint.h>"],
+      [test "$ac_cv_header_inttypes_h" = "yes"], 
[INCLUDE_STDINT="#include <inttypes.h>"],
+      [test "$ac_cv_header_sys_types_h" = "yes"],
[INCLUDE_STDINT="#include <sys/types.h>"])
+
+AC_SUBST([INCLUDE_STDINT])
+
 AC_CONFIG_FILES([
            Makefile libspeexdsp/Makefile doc/Makefile SpeexDSP.spec
            include/Makefile include/speex/Makefile speexdsp.pc
diff --git a/include/speex/speexdsp_config_types.h.in
b/include/speex/speexdsp_config_types.h.in
index 02b82fd..5ea7b55 100644
--- a/include/speex/speexdsp_config_types.h.in
+++ b/include/speex/speexdsp_config_types.h.in
@@ -1,13 +1,7 @@
 #ifndef __SPEEX_TYPES_H__
 #define __SPEEX_TYPES_H__
 
-#if defined HAVE_STDINT_H
-#  include <stdint.h>
-#elif defined HAVE_INTTYPES_H
-#  include <inttypes.h>
-#elif defined HAVE_SYS_TYPES_H
-#  include <sys/types.h>
-#endif
+ at INCLUDE_STDINT@
 
 typedef @SIZE16@ spx_int16_t;
 typedef @USIZE16@ spx_uint16_t;
-- 
2.1.4

On Sun, Jul 5, 2015 at 11:10 AM, Tanu Kaskinen <tanuk at iki.fi>
wrote:
> From: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
>
> Not everyone who includes speexdsp_config_types.h will have a test
> which defines those, and if we've chosen to use the stdint types at
> configure time then we know exactly which header(s) are available, so
> just choose the best one then and generate the header to use it.
>
> This patch, including the above text, is copied from a commit in the
> speex repository[1]. The original commit for speex was made by Ron
> <ron at debian.org>.
Merged, thanks.

Best,
Tristan

FTR, my main concern with this kind of approach is the case where your
platform has two compilers, only one of which has stdint.h

	Jean-Marc

On 07/05/2015 11:10 AM, Tanu Kaskinen wrote:
> From: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
> 
> Not everyone who includes speexdsp_config_types.h will have a test
> which defines those, and if we've chosen to use the stdint types at
> configure time then we know exactly which header(s) are available, so
> just choose the best one then and generate the header to use it.
> 
> This patch, including the above text, is copied from a commit in the
> speex repository[1]. The original commit for speex was made by Ron
> <ron at debian.org>.
> 
> [1]
https://git.xiph.org/?p=speex.git;a=commitdiff;h=774c87d6cb7dd8dabdd17677fc6da753ecf4aa87
> 
> Signed-off-by: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
> ---
>  configure.ac                             | 6 ++++++
>  include/speex/speexdsp_config_types.h.in | 8 +-------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 2cd2d1e..1de0c23 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -334,6 +334,12 @@ AC_SUBST([USIZE16])
>  AC_SUBST([SIZE32])
>  AC_SUBST([USIZE32])
>  
> +AS_IF([test "$ac_cv_header_stdint_h" = "yes"],   
[INCLUDE_STDINT="#include <stdint.h>"],
> +      [test "$ac_cv_header_inttypes_h" = "yes"], 
[INCLUDE_STDINT="#include <inttypes.h>"],
> +      [test "$ac_cv_header_sys_types_h" = "yes"],
[INCLUDE_STDINT="#include <sys/types.h>"])
> +
> +AC_SUBST([INCLUDE_STDINT])
> +
>  AC_CONFIG_FILES([
>             Makefile libspeexdsp/Makefile doc/Makefile SpeexDSP.spec
>             include/Makefile include/speex/Makefile speexdsp.pc
> diff --git a/include/speex/speexdsp_config_types.h.in
b/include/speex/speexdsp_config_types.h.in
> index 02b82fd..5ea7b55 100644
> --- a/include/speex/speexdsp_config_types.h.in
> +++ b/include/speex/speexdsp_config_types.h.in
> @@ -1,13 +1,7 @@
>  #ifndef __SPEEX_TYPES_H__
>  #define __SPEEX_TYPES_H__
>  
> -#if defined HAVE_STDINT_H
> -#  include <stdint.h>
> -#elif defined HAVE_INTTYPES_H
> -#  include <inttypes.h>
> -#elif defined HAVE_SYS_TYPES_H
> -#  include <sys/types.h>
> -#endif
> + at INCLUDE_STDINT@
>  
>  typedef @SIZE16@ spx_int16_t;
>  typedef @USIZE16@ spx_uint16_t;
>

On Mon, Jul 06, 2015 at 10:39:32AM -0400, Jean-Marc Valin
wrote:
> FTR, my main concern with this kind of approach is the case where your
> platform has two compilers, only one of which has stdint.h
It was a while ago now, so I might be forgetting something, but the
rationale for this went something like:

In ef80120166c3a2552f77008f40c59a84577a36b5 we switched to preferring
the stdint types if they were available at configure time for the lib
build.  This fixed the case where your platform has two compilers, one
of which thinks long has 32bits and one which thinks it has 64bits or
other similar variations to the standard types - which is becoming an
increasingly common situation.

That wasn't just a theoretical issue, it meant the generated headers
were different in situations where they didn't need to be, which broke
the ability to easily use speex (and ogg before we fixed it there too)
in multi-arch systems.


Someone then reported their app failing to build, because it wasn't
using configure and/or exporting HAVE_STDINT_H, so the needed header
wasn't included by the conditional construct - but since we knew the
right header at configure time, that was 'easy' to fix.


I agree you've highlighted yet another pathological case that might be
real on some systems too - but it's less clear to me that the lib build
could usefully be shared on such a system.  stdint.h is part of libc,
not of the compiler, and if you're using the same lib build with a
totally different toolchain and libc/libm, there's scope for all sorts
of hilarity to ensue depending on the precise details of that.

I think if we hit a real case of that we should look at how we can
address it, but the only ones I can think of right now are where the
toolchains are so different that you'd be using separate builds of
all of your dependencies anyway.  Is there an obvious one I'm missing
where you would want to (and be able to) share this between them?

  Ron

> On 07/05/2015 11:10 AM, Tanu Kaskinen wrote:
> > From: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
> > 
> > Not everyone who includes speexdsp_config_types.h will have a test
> > which defines those, and if we've chosen to use the stdint types
at
> > configure time then we know exactly which header(s) are available, so
> > just choose the best one then and generate the header to use it.
> > 
> > This patch, including the above text, is copied from a commit in the
> > speex repository[1]. The original commit for speex was made by Ron
> > <ron at debian.org>.
> > 
> > [1]
https://git.xiph.org/?p=speex.git;a=commitdiff;h=774c87d6cb7dd8dabdd17677fc6da753ecf4aa87
> > 
> > Signed-off-by: Tanu Kaskinen <tanu.kaskinen at linux.intel.com>
> > ---
> >  configure.ac                             | 6 ++++++
> >  include/speex/speexdsp_config_types.h.in | 8 +-------
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 2cd2d1e..1de0c23 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -334,6 +334,12 @@ AC_SUBST([USIZE16])
> >  AC_SUBST([SIZE32])
> >  AC_SUBST([USIZE32])
> >  
> > +AS_IF([test "$ac_cv_header_stdint_h" = "yes"],   
[INCLUDE_STDINT="#include <stdint.h>"],
> > +      [test "$ac_cv_header_inttypes_h" = "yes"], 
[INCLUDE_STDINT="#include <inttypes.h>"],
> > +      [test "$ac_cv_header_sys_types_h" = "yes"],
[INCLUDE_STDINT="#include <sys/types.h>"])
> > +
> > +AC_SUBST([INCLUDE_STDINT])
> > +
> >  AC_CONFIG_FILES([
> >             Makefile libspeexdsp/Makefile doc/Makefile SpeexDSP.spec
> >             include/Makefile include/speex/Makefile speexdsp.pc
> > diff --git a/include/speex/speexdsp_config_types.h.in
b/include/speex/speexdsp_config_types.h.in
> > index 02b82fd..5ea7b55 100644
> > --- a/include/speex/speexdsp_config_types.h.in
> > +++ b/include/speex/speexdsp_config_types.h.in
> > @@ -1,13 +1,7 @@
> >  #ifndef __SPEEX_TYPES_H__
> >  #define __SPEEX_TYPES_H__
> >  
> > -#if defined HAVE_STDINT_H
> > -#  include <stdint.h>
> > -#elif defined HAVE_INTTYPES_H
> > -#  include <inttypes.h>
> > -#elif defined HAVE_SYS_TYPES_H
> > -#  include <sys/types.h>
> > -#endif
> > + at INCLUDE_STDINT@
> >  
> >  typedef @SIZE16@ spx_int16_t;
> >  typedef @USIZE16@ spx_uint16_t;
> > 
> _______________________________________________
> Speex-dev mailing list
> Speex-dev at xiph.org
> http://lists.xiph.org/mailman/listinfo/speex-dev