bswap16 and bswap32 defined on NetBSD

[Michael Piotrowski]

Hi,

I just tried to build the current git version on NetBSD 5.0.2 and ran
into the following problem:

--8<---------------cut here---------------start------------->8---
CC ape.o
In file included from ape.c:11:
utils.h:140: error: static declaration of 'bswap16' follows non-static declaration
/usr/include/sys/bswap.h:20: error: previous declaration of 'bswap16' was here
utils.h:153: error: static declaration of 'bswap32' follows non-static declaration
/usr/include/sys/bswap.h:21: error: previous declaration of 'bswap32' was here
gmake: *** [ape.o] Error 1
--8<---------------cut here---------------end--------------->8---

I currently don't have the time to look into it and produce a patch, but
maybe someone else does ;-)

Thanks and best regards

[Johannes Weißl]

* byte swapping macros are now bswap16 and bswap32
* byte swapping functions are now swap_uint16 and swap_uint32

This way it is more obvious that they are actually functions that cast
their argument, so there is less confusion.

Reported-by: Michael Piotrowski <***@dynalabs.de>
---
flac.c | 4 ++--
player.c | 8 ++++----
utils.h | 43 +++++++++++++++++++++++++++----------------
3 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/flac.c b/flac.c
index 7bdf0ec..eec9e60 100644
--- a/flac.c
+++ b/flac.c
@@ -171,8 +171,8 @@ static int eof_cb(const Dec *dec, void *data)

#if defined(WORDS_BIGENDIAN)

-#define LE16(x) bswap16(x)
-#define LE32(x) bswap32(x)
+#define LE16(x) swap_uint16(x)
+#define LE32(x) swap_uint32(x)

#else

diff --git a/player.c b/player.c
index 33ca97b..66616ef 100644
--- a/player.c
+++ b/player.c
@@ -167,7 +167,7 @@ static const unsigned short soft_vol_db[100] = {

static inline void scale_sample_int16_t(int16_t *buf, int i, int vol, int swap)
{
- int32_t sample = swap ? (int16_t)bswap16(buf[i]) : buf[i];
+ int32_t sample = swap ? (int16_t)swap_uint16(buf[i]) : buf[i];

if (sample < 0) {
sample = (sample * vol - SOFT_VOL_SCALE / 2) / SOFT_VOL_SCALE;
@@ -178,12 +178,12 @@ static inline void scale_sample_int16_t(int16_t *buf, int i, int vol, int swap)
if (sample > INT16_MAX)
sample = INT16_MAX;
}
- buf[i] = swap ? bswap16(sample) : sample;
+ buf[i] = swap ? swap_uint16(sample) : sample;
}

static inline void scale_sample_int32_t(int32_t *buf, int i, int vol, int swap)
{
- int64_t sample = swap ? (int32_t)bswap32(buf[i]) : buf[i];
+ int64_t sample = swap ? (int32_t)swap_uint32(buf[i]) : buf[i];

if (sample < 0) {
sample = (sample * vol - SOFT_VOL_SCALE / 2) / SOFT_VOL_SCALE;
@@ -194,7 +194,7 @@ static inline void scale_sample_int32_t(int32_t *buf, int i, int vol, int swap)
if (sample > INT32_MAX)
sample = INT32_MAX;
}
- buf[i] = swap ? bswap32(sample) : sample;
+ buf[i] = swap ? swap_uint32(sample) : sample;
}

static inline int sf_need_swap(sample_format_t sf)
diff --git a/utils.h b/utils.h
index ac3a1d7..8b95dc9 100644
--- a/utils.h
+++ b/utils.h
@@ -136,30 +136,41 @@ static inline int is_freeform_true(const char *c)
c[0] == 't' || c[0] == 'T';
}

-static inline uint16_t bswap16(uint16_t u)
-{
-#if defined(bswap_16)
- /* GNU libc */
- return bswap_16(u);
+/* e.g. NetBSD */
+#if defined(bswap16)
+/* GNU libc */
+#elif defined(bswap_16)
+# define bswap16 bswap_16
+/* e.g. OpenBSD */
#elif defined(swap16)
- /* e.g. OpenBSD */
- return swap16(u);
+# define bswap16 swap16
#else
- return (u << 8) | (u >> 8);
+# define bswap16(x) \
+ ((((x) >> 8) & 0xff) | (((x) & 0xff) << 8))
#endif
-}

-static inline uint32_t bswap32(uint32_t u)
+static inline uint16_t swap_uint16(uint16_t x)
{
-#if defined(bswap_32)
- /* GNU libc */
- return bswap_32(u);
+ return bswap16(x);
+}
+
+/* e.g. NetBSD */
+#if defined(bswap32)
+/* GNU libc */
+#elif defined(bswap_32)
+# define bswap32 bswap_32
+/* e.g. OpenBSD */
#elif defined(swap32)
- /* e.g. OpenBSD */
- return swap32(u);
+# define bswap32 swap32
#else
- return (u >> 24) | ((u & 0xff0000) >> 8) | ((u & 0xff00) << 8) | (u << 24);
+# define bswap32(x) \
+ ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) | \
+ (((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24))
#endif
+
+static inline uint32_t swap_uint32(uint32_t x)
+{
+ return bswap32(x);
}

static inline uint32_t read_le32(const char *buf)

[Johannes Weißl]

Hello Michael,

Post by Michael Piotrowski
I just tried to build the current git version on NetBSD 5.0.2 and ran
--8<---------------cut here---------------start------------->8---
CC ape.o
utils.h:140: error: static declaration of 'bswap16' follows non-static declaration
/usr/include/sys/bswap.h:20: error: previous declaration of 'bswap16' was here
utils.h:153: error: static declaration of 'bswap32' follows non-static declaration
/usr/include/sys/bswap.h:21: error: previous declaration of 'bswap32' was here
gmake: *** [ape.o] Error 1
--8<---------------cut here---------------end--------------->8---
I currently don't have the time to look into it and produce a patch, but
maybe someone else does ;-)

I caused the error, so it's up to me to write the patch :-). I tested it
on OpenBSD, so I thought the other BSDs should be fine... mistake!
Does it work with this patch?


Greetings,
Johannes

[Michael Piotrowski]

Hi Johannes,

Thanks for the quick reply.

Post by Johannes Weißl
I caused the error, so it's up to me to write the patch :-).

:-)

Post by Johannes Weißl
I tested it on OpenBSD, so I thought the other BSDs should be
fine... mistake! Does it work with this patch?

Yes, it compliles cleanly now and works fine, too. I guess this could
be integrated into the master branch.

Thanks again!

[Gregory Petrosyan]

Post by Johannes Weißl
I tested it on OpenBSD, so I thought the other BSDs should be
fine... mistake!  Does it work with this patch?

Yes, it compliles cleanly now and works fine, too.  I guess this could
be integrated into the master branch.

Yeah, definitely — merged!

                Gregory