[Matroska-devel] [Patches] Portability, support for CloudABI, etc.
ed at nuxi.nl
Tue Nov 24 12:45:00 CET 2015
2015-11-21 11:39 GMT+01:00 Moritz Bunkus <moritz at bunkus.org>:
> 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.
I have to confess I'm not a C++ ABI expert, but how does adding an
additional constructor break the ABI? Adding additional virtual
functions will break its vtable, but as far as I know, adding an extra
constructor would do nothing in addition to exporting another symbol,
> 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"
> This is not acceptable, but totally understandable, and all programs
> using autoconf and libEBML will run into this.
Ah, yes. I didn't think about that. config.h defines all sorts of
random things that clutter up the global namespace, of course.
> 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
This is interesting. Such a feature was already requested back in 1999:
But it looks as if nothing like that was ever added. Packages like
GNOME's glib still generate their glibconfig.h header file manually
(~200 lines of m4/shell scripting).
To be honest, I don't think it's worth investing that amount of effort
in this case. We coulkd consider just adding the following to the
Alternatively, I wouldn't mind holding on to the patch myself for the
time being. Adding the FILE*-based constructor would be nice, though.
Ed Schouten <ed at nuxi.nl>
Nuxi, 's-Hertogenbosch, the Netherlands
More information about the Matroska-devel