| Message ID | 1453393016-20734-1-git-send-email-suraev@alumni.ntnu.no |
|---|---|
| State | Accepted |
| Headers | show |
> On 21 Jan 2016, at 17:16, suraev@alumni.ntnu.no wrote: > > From: Max <msuraev@sysmocom.de> > > Mark unsigned value as such. > Fix unaligned access error revealed by asan on 32 bit builds. Linux vagrant-ubuntu-wily-64 4.2.0-23-generic #28-Ubuntu SMP Sun Dec 27 17:47:31 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
> On 21 Jan 2016, at 17:16, suraev@alumni.ntnu.no wrote: > > > -void osmo_revbytebits_buf(uint8_t *buf, int len); > +void osmo_revbytebits_buf(uint8_t *buf, unsigned int len); yes that makes sense but please do not mix bugfix and API change in one (unless the API change is the bugfix). E.g. 2/3 of the change is "noise" > @@ -221,8 +220,7 @@ void osmo_revbytebits_buf(uint8_t *buf, int len) > } > > for (i = unaligned_cnt; i + 3 < len; i += 4) { > - uint32_t *cur = (uint32_t *) (buf + i); > - *cur = osmo_revbytebits_32(*cur); > + osmo_store32be(osmo_revbytebits_32(osmo_load32be(buf + i)), buf + i); uint32_t cur; memcpy(&cur, buf + 1, sizeof(cur)); cur = osmo_revbytebits_32(cur); memcpy(buf + 1, &cur, sizeof(cur)); would be my approach. So let's compare it. Your code without the loop is here https://goo.gl/vD3kqQ and the memcpy variant is https://goo.gl/5kzTmx now if there would be a cloud microbenchmark we could resolve that once and for all.
diff --git a/include/osmocom/core/bits.h b/include/osmocom/core/bits.h index 1587b05..d559185 100644 --- a/include/osmocom/core/bits.h +++ b/include/osmocom/core/bits.h @@ -75,7 +75,7 @@ uint32_t osmo_revbytebits_32(uint32_t x); uint32_t osmo_revbytebits_8(uint8_t x); /* \brief reverse the bits of each byte in a given buffer */ -void osmo_revbytebits_buf(uint8_t *buf, int len); +void osmo_revbytebits_buf(uint8_t *buf, unsigned int len); /*! \brief left circular shift * \param[in] in The 16 bit unsigned integer to be rotated diff --git a/src/bits.c b/src/bits.c index 01d7e73..e90bb71 100644 --- a/src/bits.c +++ b/src/bits.c @@ -206,10 +206,9 @@ uint32_t osmo_revbytebits_8(uint8_t x) * * This function reverses the bits in each byte of the buffer */ -void osmo_revbytebits_buf(uint8_t *buf, int len) +void osmo_revbytebits_buf(uint8_t *buf, unsigned int len) { - unsigned int i; - unsigned int unaligned_cnt; + unsigned int i, unaligned_cnt; int len_remain = len; unaligned_cnt = ((unsigned long)buf & 3); @@ -221,8 +220,7 @@ void osmo_revbytebits_buf(uint8_t *buf, int len) } for (i = unaligned_cnt; i + 3 < len; i += 4) { - uint32_t *cur = (uint32_t *) (buf + i); - *cur = osmo_revbytebits_32(*cur); + osmo_store32be(osmo_revbytebits_32(osmo_load32be(buf + i)), buf + i); len_remain -= 4; }
From: Max <msuraev@sysmocom.de> Mark unsigned value as such. Fix unaligned access error revealed by asan on 32 bit builds. Sponsored-by: On-Waves ehf --- include/osmocom/core/bits.h | 2 +- src/bits.c | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-)