#360 cmake build fixes, mostly related to O_LARGEFILE redefinition

[Ozkan Sezer]

(1) cmake: config.h defining O_LARGEFILE is broken: config.h is usually
included before everything else, so it results in redefinition errors.
Attached patch removes it and adjusts autotools too for good symmetry.

(2) cmake: check for O_LARGEFILE is broken and always fails. Attached
patch fixes it.

(3) cmake: Request C99. Necessary with older gcc which have c99 support
but just not by default.

(4) cmake: WITH_SSE description is hard to read. Arguably better one in
the attached patch.

Note for (1) and (2) above: Do you really need to use O_LARGEFILE flag?
The open(2) man page (glibc) suggest setting _FILE_OFFSET_BITS to 64.
instead. In any case, that'd be another discussion.

[Thomas Orgis]

(1) Well, it is only defined after the configure test determined that it won't be defined later, anyway. But I see the point that a HAVE guard is cleaner. I'll change the method accordingly.

(2) Oh, yes, that code is a bit too reduced. Broken in any case. But still, it's nasty that the function is named check_c_source_compiles, not check_c_source_compiles_and_links_as_executable.

(3) Yes, C99 is intended. Thanks.

(4) Hm, I think it also needs a hint about autodetection.

I worked on the above points now, please see current trunk.

About the O_LARGEFILE discussion: Yes, I am rather annoyed how its use is discouraged. The whole mess with adding new API avoiding off_t is because of the broken idea of _FILE_OFFSET_BITS. My point using lseek64() and O_LARGEFILE explicitly in lfs_wrap.c is that I need to implement both off_t (32 bit) and off64_t API in there. Want all that mess contained in one file and I want to be sure in the whole library codebase what off_t means. I want to know that I use 64 bit offset API. This is the way to make sure.

The end-user programs still use plain off_t and _FILE_OFFSET_BITS, as normal user code indeed shouldn't worry more about that than perhaps setting that flag. But for offering a library API that needs to anticipate the user setting that or not it is a total clusterfuck, which kept me busy time again and again for years.

Thanks for your valid points here, though. I hope to get this finally settled, again. How is the current state of trunk working out for you?

[Ozkan Sezer]

Svn-trunk looks good.

The cmake side doesn't reflect autotools wrt --disable-components & co: configure with -DBUILD_LIBOUT123=OFF (I couldn't see other options) for windows with a toolchain w/o GetThreadErrorMode/SetThreadErrorMode, it doesn't check for those two funcs and doesn't disable modules either, and linkage fails as a result. (Just reporting though, I won't fight for it, I use autotools with ease.)

[Thomas Orgis]

I pushed 1.32.2 release now with the 4 points … about CMake build not being on par with the rest, that's known. Feature parity is no goal that I work on. The autoconf build is the main one. About missing checks and broken build … do you have such a toolchain at hand and can check the current trunk if it disables modules?

[Ozkan Sezer]

The toolchain I have is really outdated, so don't worry about it. Feel free to close this ticket.

[Ozkan Sezer]

The current trunk seems to work around the GetThreadErrorMode fine.
I suggest the following, to make things final.

diff --git a/src/libmpg123/lfs_wrap.c b/src/libmpg123/lfs_wrap.c
index 1474de7..e082bb7 100644
--- a/src/libmpg123/lfs_wrap.c
+++ b/src/libmpg123/lfs_wrap.c
@@ -48,4 +48,8 @@
 #include "debug.h"

+#if defined(_WIN32) && !defined(EOVERFLOW)
+#define EOVERFLOW 132
+#endif
+
 // We do not want to expose this publicly, but it is cleaner to have it also defined
 // as portable API to offer the legacy function wrapper over. It's an undocumented

[Thomas Orgis]

Are errno values macros under Windows? I guess defining to the documented value won't hurt ...

sent from mobile device, trustworthy or not

[Ozkan Sezer]

Yes. (And not as enum, if that's what you're worried about.)

[Ozkan Sezer]

This can and should be closed now.