GNOME Bugzilla – Bug 501830
gmime API uses off_t - may break ABI
Last modified: 2008-05-31 16:32:13 UTC
Using of off_t in the public API makes possible to build two equally-looking gmime libraries with the same soname, and each of them will have a different ABI. It will break binaries compiled against gmime, whenever package maintainer changes --enable-largefile flag value and also breaks compatibility of binaries between distributions. In both cases, this change causes no warning. Ugly hacks introduced by a possibility to compile glib and gmime with different value of large file support caused other breakages: pkg-config, gmime-config and gmimeConf.sh breakage: https://bugzilla.novell.com/show_bug.cgi?id=344662 Incorrect handling breaks obscure combinations in existing gtk# versions: https://bugzilla.novell.com/show_bug.cgi?id=319824 Forcing value for off_t in config file may also introduce badly debuggable crashes in the code using gmime. That is why you should do one of following changes: 1. Replace off_t by any other type which is not dependent on C flags in all (or all exported) functions and increase soname. or 2. Exhibit large file support in library name and build libgmime-off_t_8-2.0.so.2 and/or libgmime-off_t_4-2.0.so.2 and fix related config/mono file problems.
Created attachment 100306 [details] [review] gmime-cflags.patch Patch #1: Remove all ugly hacks related to off_t.
Created attachment 100313 [details] [review] gmime-no-off_t-api.patch After previous patch, call mv mono/GMime.metadata.in mono/GMime.metadata. (Note that previous patch turns large files support on by default unless specified otherwise to configure). Incremental patch #2: Remove off_t from public interfaces (except those declared as private in the header). Note: Some of remaining off_t types are declared as private, but they are used by tests from directory tests, so it is not possible to remove OFF_T. Note: Patch increments soname in an ugly way to not break your versioning scheme. Best would be following of recommended libtool versioning (libtool.info: Updating library version information) starting with 3:0:0.
Created attachment 100323 [details] [review] gmime-no-off_t-at-all.patch Optional incremental patch #3 replacing all remaining occurrences of off_t by goffset and removing rest of off_t related magic. Note: gmime-api.xml and documentation has to be regenerated. Its regeneration might depend on an exact way of fix of https://bugzilla.novell.com/show_bug.cgi?id=319824 If goffset will not be added, you have to use long instead of goffset in the binding metadata.
http://bugs.debian.org/443561 is the Debian edition of this bug.
> It will break binaries compiled against gmime, whenever package maintainer > changes --enable-largefile flag value right, so don't change the default value (which is 'disabled') :) > and also breaks compatibility of binaries > between distributions. In both cases, this change causes no warning. only if package maintainers change the default. I honestly think the best solution to this problem is to simply not enable LFS since our goal is to maintain API/ABI compat, correct? That said, I should have added a prominent WARNING message in the configure script when you --enable-largefile in order to make sure the user/package maintainer is aware of the problems with doing so. I've just added a WARNING message to svn's configure script to make this problem a bit more obvious in the future. When I begin work on 3.0, I will make sure to use a 64bit offset variable (goffset?) always, even if LFS is unavailable on the platform (I'll be making LFS enabled by default for 3.0)
No, disable it by default is not a solution. Imagine an application that enables large file support and includes gmime header files compiled with the default (--disable-largefile). This application will push 64 bits on stack for all off_t values, gmime will pop only 32 bits => crash. Disable it by default without patching is even impossible for distros, which already enabled it, because it will cause ABI breakage. They would need to increment soname. And please keep in mind, that for example in OpenSUSE it is considered as a bug to not enable large file support in packages that support it. Possible solutions: 1. I can see only two work-arounds not breaking ABI: 1.1 Headers should check for large file support and fail if it is turned on by the application including gmime. Very ugly - it will limit file sizes to 2G in applications using gmime. 1.2 Expand all off_t to static type in time of compilation. You can do it with my second patch by replacing goffset by @off_t@ and making all affected .c and .h files generated by configure. Ugly to implement, less ugly to use. 2. Break ABI keeping API: 2.1 Apply my patches (at least 1 and 2). It keeps API and soname is incremented, it seems to be more safe than now, when you can have two ABI incompatible instances with the same soname. Yes, glib now uses 64 bits for offset for all platforms, that support 64 bit integers, even if large file support is disabled. The only new item in GLib 2.14 is the goffset typedef.
> Imagine an application that enables large file support and includes gmime > header files compiled with the default (--disable-largefile). This application > will push 64 bits on stack for all off_t values, gmime will pop only 32 bits => > crash. this is no different from older versions of gmime (e.g. up-to-and-including 2.2.9) and no one seemed to complain then > Disable it by default without patching is even impossible for distros, which > already enabled it, because it will cause ABI breakage. They would need to > increment soname. then the ABI is already broken, so keeping things the way they are doesn't break anything. you're making a mountain out of a mole hill. If SuSE already ships GMime with LFS and you claim that SuSE enables LFS for everything, then all apps built on that GMime package will also have LFS... and any app that joe-random-user builds using GMime will get LFS for free as well (did you notice that `pkg-config --cflags gmime-2.0` gives you all the -D compile flags to enable LFS?) So this really isn't an issue.
Also, this has caused breakage in the Mono bindings, which don't (right now) have any knowledge of large file support. Methods which use off_t are mapped to IntPtr, which is 64 bits on 64-bit systems and 32 bits on 32-bit systems. Normally, that's all good. But when large file support is enabled, it should be 64 bits on a 32-bit system. It isn't, and things crash -- in particular, a failed assertion from GMime's stream_seek() method is common. Maybe this should be a separate bug?
See this for an example of the Mono problem in Beagle: http://linux.derkeiler.com/Newsgroups/alt.os.linux.suse/2008-02/msg00790.html
Joe: I thought the GMime Mono bindings issue had been resolved?
oh yeah, I forgot that I fixed this back in September. dBera, do you know if they are using a version older than 2.2.11 (the first release which had the fix)?
Recently jpr faced the same problem and I am pretty sure he is using a fairly gmrecent gmime. The link from #9 (and one other problem reported by an OpenSUSE factory user a month ago) suggests that the problem exists (at least) with recent gmime on opensuse. The only thing I know is debian patches gmime differently than what Joe did. They replace off_t by "long long" instead of "gint64". Now I am not sure if gint64 maps to long long or unsigned long long and under which machine conditions. Maybe its a red herring or maybe there is something there.
Anything that has not variable size is a correct fix. The correct type is goffset, but it requires fairly new GLib.
Created attachment 108409 [details] [review] gmime-no-off_t.patch Patch for gmime-2.2.18. To prevent future large rejects, patch is done differently: First run this script: # Replace all occurrences of variable sized off_t by goffset: sed -i "s/off_t/goffset/g;s/OFF_T/G_GINT64_FORMAT/g" */*.[ch] # GMime.metadata does not need to be generated any more mv mono/GMime.metadata.in mono/GMime.metadata # api file is not regenerated during build. Fix it now: # Current gtk# does not support goffset yet. sed -i s/off_t/long/g mono/gmime-api.raw # And finally fix all remaining problems related to off_t: patch -p0 <gmime-no-off_t.patch
I'm working on a 2.3/2.4 branch that will resolve this issue
fixed in the 2.3.x series