On Wed, 27 Nov 2013, Diego Biurrun wrote:
On Wed, Nov 27, 2013 at 10:15:09PM +0200, Martin Storsjö wrote:
On Wed, 27 Nov 2013, Diego Biurrun wrote:
--- a/configure
+++ b/configure
@@ -3355,45 +3380,52 @@ esac
# determine libc flavour
-# uclibc defines __GLIBC__, so it needs to be checked before glibc.
-if check_cpp_condition features.h "defined __UCLIBC__"; then
- libc_type=uclibc
- add_cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
-elif check_cpp_condition features.h "defined __GLIBC__"; then
- libc_type=glibc
- add_cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
-# MinGW headers can be installed on Cygwin, so check for newlib first.
-elif check_cpp_condition newlib.h "defined _NEWLIB_VERSION"; then
- libc_type=newlib
- add_cppflags -U__STRICT_ANSI__
-elif check_header _mingw.h; then
- libc_type=mingw
- check_cpp_condition _mingw.h \
- "defined (__MINGW64_VERSION_MAJOR) || (__MINGW32_MAJOR_VERSION > 3) ||
\
- (__MINGW32_MAJOR_VERSION == 3 && __MINGW32_MINOR_VERSION >= 15)" ||
- die "ERROR: MinGW runtime version must be >= 3.15."
- add_cppflags -U__STRICT_ANSI__
-elif check_func_headers stdlib.h _get_doserrno; then
- libc_type=msvcrt
- add_compat strtod.o strtod=avpriv_strtod
- add_compat msvcrt/snprintf.o snprintf=avpriv_snprintf \
- _snprintf=avpriv_snprintf \
- vsnprintf=avpriv_vsnprintf
- # The MSVC 2010 headers (Win 7.0 SDK) set _WIN32_WINNT to
- # 0x601 by default unless something else is set by the user.
- # This can easily lead to us detecting functions only present
- # in such new versions and producing binaries requiring windows 7.0.
- # Therefore explicitly set the default to XP unless the user has
- # set something else on the command line.
- check_cpp_condition stdlib.h "defined(_WIN32_WINNT)" || add_cppflags
-D_WIN32_WINNT=0x0502
-elif check_cpp_condition stddef.h "defined __KLIBC__"; then
- libc_type=klibc
-elif check_cpp_condition sys/cdefs.h "defined __BIONIC__"; then
- libc_type=bionic
- add_compat strtod.o strtod=avpriv_strtod
-fi
+probe_libc(){
+ pfx=$1
+ # uclibc defines __GLIBC__, so it needs to be checked before glibc.
+ if check_${pfx}cpp_condition features.h "defined __UCLIBC__"; then
+ eval ${pfx}libc_type=uclibc
+ add_${pfx}cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
+ elif check_${pfx}cpp_condition features.h "defined __GLIBC__"; then
+ eval ${pfx}libc_type=glibc
+ add_${pfx}cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
+ # MinGW headers can be installed on Cygwin, so check for newlib first.
+ elif check_${pfx}cpp_condition newlib.h "defined _NEWLIB_VERSION"; then
+ eval ${pfx}libc_type=newlib
+ add_${pfx}cppflags -U__STRICT_ANSI__
+ elif check_${pfx}header _mingw.h; then
+ eval ${pfx}libc_type=mingw
+ mingw_condition="defined (__MINGW64_VERSION_MAJOR) || (__MINGW32_MAJOR_VERSION > 3) ||
(__MINGW32_MAJOR_VERSION == 3 && __MINGW32_MINOR_VERSION >= 15)"
+ check_${pfx}cpp_condition _mingw.h "defined $mingw_condition " ||
+ die "ERROR: MinGW runtime version must be >= 3.15."
Any technical reason to split out mingw_condition into a separate
variable here, or is it just an unrelated refactoring you're doing
at the same time (which raises a few questions for a reviewer)? If
there's no pressing need for it (keeping lines short is not enough,
when it comes to a nontrivial change as this one), I'd rather have
it kept as before, to reduce the amount of changes here. It can
always be refactored in a separate commit with an adequate
explanation.
I /think/ it failed back when I tested this on MinGW, but I have to
test again now. I just rebased on master and tested under Linux to
get some initial comments, hence the [RFC]. Otherwise I completely
agree with you in general.
Ok, needs to be investigated at least.
+ add_${pfx}cppflags -U__STRICT_ANSI__
+ elif check_${pfx}func_headers stdlib.h _get_doserrno; then
+ eval ${pfx}libc_type=msvcrt
+ add_compat strtod.o strtod=avpriv_strtod
+ add_compat msvcrt/snprintf.o snprintf=avpriv_snprintf \
+ _snprintf=avpriv_snprintf \
+ vsnprintf=avpriv_vsnprintf
Things like add_compat here (and below) should only be done if this
is the target libc, not if it's the host one. In general, every
single action (add/set/whatever) in this needs to include $pfx in
one way or another, otherwise it's doing something wrong.
When using msvc as host compiler this is not necessary?
Well, add_compat only adds the objects to be used in target complication
anyway, so calling that based on the host libc detection is just broken
anyway.
Apparently we've been able to cope just fine without these with MSVC as
host compiler so far. They are only added in the target compilation to
amend the existing strtod/snprintf implementations to comply with more
corner cases that we don't exercise in the host tools. For strtod, it's
about parsing hex literal strings (which strtol can handle but not strtod
in both msvcrt and bionic) - and I don't think any of the host tools use
that feature. For snprintf, it's about getting the right semantics in case
the buffer overflows (the msvcrt implementation doesn't null terminate it
in this case, which the C standard says it should). So for target
compilation, it's needed because otherwise a whole lot of string handling
suddenly would be insecure. For host building, you're not (quite as much
at least) exposed to the threat of any random incoming data, but only to
the threat of what's in the libav source itself.
So unless there's some real need, I wouldn't try to expand the add_compat
framework for host building yet. (Hint: It's a little more work than you'd
expect - you'd need to build these object files as compat/strtod_host.o or
so, to separate them from the compat objs for the target.)
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
