search

janbar / noson

C++ library for accessing SONOS devices.
GNU General Public License v3.0
27 stars 9 forks source link

swap* routines conflict with OpenBSD swap(3) macros #20

Closed klemensn closed 5 months ago

klemensn commented 5 months ago

https://man.openbsd.org/swap16.3

/usr/ports/pobj/noson-2.12.6/noson-2.12.6/noson/src/private/byteorder.h:57:23: error: redefinition of '__uint16_t' as different kind of symbol
static inline int16_t swap16(int16_t val)
                      ^

I haven't looked as to why noson needs its own routines for this and/or which one should be used, but a simple fix to unbreak the build is prepending #undef swap16 to the function definition in byteorder.h -- same problem for swap32().

Full build failure log, in case that helps: https://gist.github.com/klemensn/cc23726ee6231c0708720905003168c2

klemensn commented 5 months ago

noson-app has the same problem.

janbar commented 5 months ago

I will have a look on this issue on BSD. In others Unix like , the function swap16 is declared in header sys/endian.h. It seems some top includes load this header on OpenBSD. The resolution could be to rename my functions or to prefix them in an anonymous namespace.

janbar commented 5 months ago

I pushed a new commit , and a new release 2.12.7. It should help to fix this issue.

klemensn commented 5 months ago

Still fails on OpenBSD/amd64 7.4-current with clang and libstd++16.0.6:

FAILED: noson/CMakeFiles/noson.dir/src/private/byteorder.cpp.o 
/usr/ports/pobj/noson-2.12.7/bin/c++ -Dnoson_EXPORTS -I/usr/ports/pobj/noson-2.12.7/build-amd64/noson/public/noson -I/usr/ports/pobj/noson-2.12.7/build-amd64/noson/public -I/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src -isystem /usr/local/include -O2 -pipe -DNDEBUG -std=gnu++11 -fPIC -MD -MT noson/CMakeFiles/noson.dir/src/private/byteorder.cpp.o -MF noson/CMakeFiles/noson.dir/src/private/byteorder.cpp.o.d -o noson/CMakeFiles/noson.dir/src/private/byteorder.cpp.o -c /usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.cpp
In file included from /usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.cpp:19:
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:1: warning: inline variables are a C++17 extension [-Wc++17-extensions]
inline int16_t swap16(int16_t val)
^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:23: error: unexpected type name 'int16_t': expected expression
inline int16_t swap16(int16_t val)
                      ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:31: error: expected ')'
inline int16_t swap16(int16_t val)
                              ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:16: note: to match this '('
inline int16_t swap16(int16_t val)
               ^
/usr/include/sys/endian.h:70:19: note: expanded from macro 'swap16'
#define swap16(x) __swap16(x)
                  ^
/usr/include/sys/_endian.h:87:35: note: expanded from macro '__swap16'
        (__uint16_t)(__builtin_constant_p(x) ? __swap16gen(x) : __swap16md(x))
                                         ^
In file included from /usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.cpp:19:
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:23: error: unexpected type name 'int16_t': expected expression
inline int16_t swap16(int16_t val)
                      ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:31: error: expected ')'
inline int16_t swap16(int16_t val)
                              ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:16: note: to match this '('
inline int16_t swap16(int16_t val)
               ^
/usr/include/sys/endian.h:70:19: note: expanded from macro 'swap16'
#define swap16(x) __swap16(x)
                  ^
/usr/include/sys/_endian.h:87:41: note: expanded from macro '__swap16'
        (__uint16_t)(__builtin_constant_p(x) ? __swap16gen(x) : __swap16md(x))
                                               ^
/usr/include/sys/_endian.h:49:31: note: expanded from macro '__swap16gen'
    (__uint16_t)(((__uint16_t)(x) & 0xffU) << 8 | ((__uint16_t)(x) & 0xff00U) >> 8)
                              ^
In file included from /usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.cpp:19:
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:23: error: unexpected type name 'int16_t': expected expression
inline int16_t swap16(int16_t val)
                      ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:31: error: expected ')'
inline int16_t swap16(int16_t val)
                              ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:16: note: to match this '('
inline int16_t swap16(int16_t val)
               ^
/usr/include/sys/endian.h:70:19: note: expanded from macro 'swap16'
#define swap16(x) __swap16(x)
                  ^
/usr/include/sys/_endian.h:87:41: note: expanded from macro '__swap16'
        (__uint16_t)(__builtin_constant_p(x) ? __swap16gen(x) : __swap16md(x))
                                               ^
/usr/include/sys/_endian.h:49:64: note: expanded from macro '__swap16gen'
    (__uint16_t)(((__uint16_t)(x) & 0xffU) << 8 | ((__uint16_t)(x) & 0xff00U) >> 8)
                                                               ^
In file included from /usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.cpp:19:
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:23: error: unexpected type name 'int16_t': expected expression
inline int16_t swap16(int16_t val)
                      ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:31: error: expected ')'
inline int16_t swap16(int16_t val)
                              ^
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:16: note: to match this '('
inline int16_t swap16(int16_t val)
               ^
/usr/include/sys/endian.h:70:19: note: expanded from macro 'swap16'
#define swap16(x) __swap16(x)
                  ^
/usr/include/sys/_endian.h:87:68: note: expanded from macro '__swap16'
        (__uint16_t)(__builtin_constant_p(x) ? __swap16gen(x) : __swap16md(x))
                                                                          ^
In file included from /usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.cpp:19:
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.h:61:35: error: expected ';' after top level declarator
inline int16_t swap16(int16_t val)
                                  ^
                                  ;
/usr/ports/pobj/noson-2.12.7/noson-2.12.7/noson/src/private/byteorder.cpp:26:13: error: no member named '__endianess__' in namespace 'SONOS'
int NSROOT::__endianess__ = test_endianess();
    ~~~~~~~~^
1 warning and 10 errors generated.
janbar commented 5 months ago

mmm, it builds without issue on FreeBSD. The problem is the OS header uses some macros to rename internal function (i.e swap16 etc), and the preprocessor apply those macros to rename my private functions. that's ugly.

janbar commented 5 months ago

I reverted the repo (noson and noson-app) to the previous tags. The issue is not in the source, but in the OS headers. The build shouldn't include "sys/endian.h".

What is the release of OpenBSD ?

klemensn commented 5 months ago

What is the release of OpenBSD ?

7.4 is the latest stable version: https://www.openbsd.org/

janbar commented 5 months ago

In header "sys/endian.h" of OpenBSD, we find:

#if __BSD_VISIBLE
#define swap16(x) __swap16(x)
#define swap32(x) __swap32(x)
#define swap64(x) __swap64(x)

Please try to build the app without the definition "__BSD_VISIBLE". Those things are not compliant. Maybe you have to specify a flag to be in "compatible mode".

janbar commented 5 months ago

I haven't looked as to why noson needs its own routines for this and/or which one should be used, but a simple fix to unbreak the build is prepending #undef swap16 to the function definition in byteorder.h -- same problem for swap32().

I cannot prepending with undef all the bunch of functions declared in the sources. The OS headers shouldn't declare macros without prefix "" for names not used in the standards (i.e printf etc). So, the flag "BSD_VISIBLE" exists to enable some special stuff of OpenBSD. Those stuff must be disabled.

Edit: Others standard declaration are needed in the section. So it shouldn't be disabled :(

Edit: Those routines aren't standard, and many OSes don't provide them. Only Linux GNU, and FreeBSD provide them, but without the same naming: Linux = bswap_16 , FreeBSD = bswap16 ... etc. For those reasons many app using byte processing (audio, video) have their own routines.

janbar commented 5 months ago

@klemensn , could you provide a PR with the required changes if you succeed to build using "ifdef/undef" ??? i.e:

#ifdef swap16
#undef swap16
#endif
...

Here the file is "src/private/byteorder.h". In the project "noson-app", the same file is in "backend/NosonMediaScanner/byteorder.h".

Thanks

janbar commented 5 months ago

I had a look about the same issue for other products, and there is a bunch of media app that get this issue on OpenBSD. The maintainers have choosen to rename their routines. So maybe it is the good choice, until the next collision ...

edit: I pushed the commit

klemensn commented 5 months ago

Thanks, both noson and noson-app now build on OpenBSD without any local patches.