[Matroska-devel] [Patches] Portability, support for CloudABI, etc.

Moritz Bunkus moritz at bunkus.org
Sat Nov 21 11:39:03 CET 2015


Hey Ed,

thanks again for the patches. I've applied and pushed the two patches to
the two libraries that set the *CodeDate variables to "Unknown".

The other patch still needs some work, though I'm not certain how to go
about it.

1. The new constructor taking a FILE*

You've unconditionally added a new constructor
StdIOCallback(FILE*). While not breaking the API it does break the
ABI. I've taken the liberty to modify your patch to only define that
constructor if fopen() is NOT available.

We can make the FILE* constructor available unconditionally the next
time we really have to break ABI compatibility.

See the attached patch for my changed version. It compiles and installs
just fine.

2. Including ebml_config.h

This is probably not that easy to solve, or I simply lack the experience
creating libraries that include their autoconf'd config.h file somehow.

I've tried compiling MKVToolNix against a libEBML that includes the v2
patch. This spits out a lot of warnings:

In file included from src/common/make_unique.h:18:0,
                 from src/common/common.h:91,
                 from src/common/common_pch.h:4,
                 from src/common/clpi.cpp:14:
./config.h:167:0: warning: "PACKAGE_TARNAME" redefined
 #define PACKAGE_TARNAME "mkvtoolnix"
 ^
In file included from
/home/mosu/tmp/libebml/include/ebml/EbmlConfig.h:39:0,
                 from
                 /home/mosu/tmp/libebml/include/ebml/EbmlTypes.h:37,
                 from
                 /home/mosu/tmp/libebml/include/ebml/EbmlElement.h:37,
                 from src/common/common.h:72,
                 from src/common/common_pch.h:4,
                 from src/common/clpi.cpp:14:
/home/mosu/tmp/libebml/include/ebml/ebml_config.h:56:0: note: this is
the location of the previous definition
 #define PACKAGE_TARNAME "libebml"
 ^
In file included from src/common/make_unique.h:18:0,
                 from src/common/common.h:91,
                 from src/common/common_pch.h:4,
                 from src/common/clpi.cpp:14:
./config.h:173:0: warning: "PACKAGE_VERSION" redefined
 #define PACKAGE_VERSION "8.5.2"
 ^
In file included from
/home/mosu/tmp/libebml/include/ebml/EbmlConfig.h:39:0,
                 from
                 /home/mosu/tmp/libebml/include/ebml/EbmlTypes.h:37,
                 from
                 /home/mosu/tmp/libebml/include/ebml/EbmlElement.h:37,
                 from src/common/common.h:72,
                 from src/common/common_pch.h:4,
                 from src/common/clpi.cpp:14:
/home/mosu/tmp/libebml/include/ebml/ebml_config.h:62:0: note: this is
the location of the previous definition
 #define PACKAGE_VERSION "1.3.3"

…

This is not acceptable, but totally understandable, and all programs
using autoconf and libEBML will run into this.

What I think the right solution could look like is having two config.h
files, a private and a public one. The private one is the config.h
that's already being created by libEBML's autoconf. It should only be
included by libEBML's .cpp files, not by its headers. This file wouldn't
have to be installed/distributed either.

The public include file (let's call it ebml_config.h) on the other hand
would have to be installed/distributed as it would be included by
libEBML's other header files. In order to minimize namespace pollution
only the absolute minimum set of pre-processor #defines should be part
of that file. Ideally these #defines would even use a specific prefix,
e.g. LIBEBML_HAVE_FOPEN.

I don't know if that's possible (or easy to do) with autoconf,
though. You seem to have (much?) more knowledge about autoconf/automake,
so maybe you have an idea how to implement this or even for something
better.

Kind regards,
mosu
-------------- next part --------------
diff --git a/Makefile.am b/Makefile.am
index 9b10ee8..a7bd4e0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -66,6 +66,9 @@ nobase_include_HEADERS = \
 	ebml/SafeReadIOCallback.h \
 	ebml/StdIOCallback.h
 
+nobase_nodist_include_HEADERS = \
+	ebml/ebml_config.h
+
 pkgconfigdir = ${libdir}/pkgconfig
 pkgconfig_DATA = libebml.pc
 
diff --git a/configure.ac b/configure.ac
index 642978c..54eddcd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
 AC_INIT([libebml], [1.3.3])
 AC_CONFIG_AUX_DIR([build-aux])
-AC_CONFIG_HEADERS([config.h])
+AC_CONFIG_HEADERS([ebml/ebml_config.h])
 AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([foreign subdir-objects tar-pax])
 AC_PROG_CXX
@@ -11,4 +11,5 @@ AC_ARG_ENABLE([debug],
 AM_CONDITIONAL([ENABLE_DEBUG], [test "$enable_debug" = yes])
 AC_CONFIG_FILES([Makefile libebml.pc])
 AC_CHECK_HEADERS([winapifamily.h])
+AC_CHECK_FUNC(fopen, AC_DEFINE(HAVE_FOPEN, [], [fopen function is present]))
 AC_OUTPUT
diff --git a/ebml/EbmlConfig.h b/ebml/EbmlConfig.h
index 5bb679f..06ec835 100644
--- a/ebml/EbmlConfig.h
+++ b/ebml/EbmlConfig.h
@@ -36,9 +36,7 @@
 #ifndef LIBEBML_CONFIG_H
 #define LIBEBML_CONFIG_H
 
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
+#include "ebml_config.h"
 
 #if defined(__linux__)
 #include <endian.h>
diff --git a/ebml/StdIOCallback.h b/ebml/StdIOCallback.h
index 65664b9..e4be232 100644
--- a/ebml/StdIOCallback.h
+++ b/ebml/StdIOCallback.h
@@ -33,6 +33,7 @@
 #ifndef LIBEBML_STDIOCALLBACK_H
 #define LIBEBML_STDIOCALLBACK_H
 
+#include "EbmlConfig.h"
 #include "IOCallback.h"
 
 #include <stdexcept>
@@ -69,8 +70,11 @@ class EBML_DLL_API StdIOCallback:public IOCallback
     uint64 mCurrentPosition;
 
     public:
-//  StdIOCallback(const char*Path,const char*Mode);
+#ifdef HAVE_FOPEN
   StdIOCallback(const char*Path, const open_mode Mode);
+#else
+  StdIOCallback(FILE*);
+#endif
   virtual ~StdIOCallback()throw();
 
   virtual uint32 read(void*Buffer,size_t Size);
diff --git a/src/StdIOCallback.cpp b/src/StdIOCallback.cpp
index f697f9b..40df7a0 100644
--- a/src/StdIOCallback.cpp
+++ b/src/StdIOCallback.cpp
@@ -60,6 +60,7 @@ CRTError::CRTError(const std::string & Description,int nError)
 }
 
 
+#ifdef HAVE_FOPEN
 StdIOCallback::StdIOCallback(const char*Path, const open_mode aMode)
 {
   assert(Path!=0);
@@ -93,6 +94,14 @@ StdIOCallback::StdIOCallback(const char*Path, const open_mode aMode)
   mCurrentPosition = 0;
 }
 
+#else  // HAVE_FOPEN
+
+StdIOCallback::StdIOCallback(FILE*Stream)
+    :File(Stream)
+{
+}
+#endif
+
 
 StdIOCallback::~StdIOCallback()throw()
 {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.matroska.org/pipermail/matroska-devel/attachments/20151121/0acfaad4/attachment-0001.sig>


More information about the Matroska-devel mailing list