From c950c6f9680085e6f6fe7fecfd5eabaef5a04af3 Mon Sep 17 00:00:00 2001 From: Matthew Krupcale Date: Mon, 9 Mar 2020 19:16:32 -0400 Subject: [PATCH] bits: grib_encode_unsigned_long: increment pointer, not the pointed-to-value This fixes a long-standing (i.e. at least since 1.9.5 ca. Oct 2010) bug in grib_encode_unsigned_long where the first byte was incorrectly incremented instead of incrementing the pointer to the next byte. This would only happen if the bit position / offset was not aligned on a byte boundary (i.e. *bitp % 8 != 0), though. Fortunately, as far as I can tell, no grib_api/eccodes functions invoked grib_encode_unsigned_long such that this bug was manifested. * src/grib_bits_any_endian.c: increment pointer, not the pointed-to-value * src/grib_bits_ibmpow.c: likewise. --- src/grib_bits_any_endian.c | 7 +------ src/grib_bits_ibmpow.c | 3 +-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/grib_bits_any_endian.c b/src/grib_bits_any_endian.c index c1bccfae9..1591fd5d1 100644 --- a/src/grib_bits_any_endian.c +++ b/src/grib_bits_any_endian.c @@ -280,12 +280,7 @@ int grib_encode_unsigned_long(unsigned char* p, unsigned long val, long* bitp, l else { tmp = ((val >> len) | ((*p) & dmasks[n])); } - *p = tmp; - (*p)++; - - /*Beware of code like this! compiler warning: operation may be undefined - Read GCC manual on -Wsequence-point*/ - /* *p = ((val << -len) | ((*p)++ & dmasks[n])); */ + *p++ = tmp; } /* write the middle words */ diff --git a/src/grib_bits_ibmpow.c b/src/grib_bits_ibmpow.c index 162a7e4a9..df2b7556a 100644 --- a/src/grib_bits_ibmpow.c +++ b/src/grib_bits_ibmpow.c @@ -107,8 +107,7 @@ int grib_encode_unsigned_long(unsigned char* p, unsigned long val, long* bitp, l else { tmp = ((val >> len) | ((*p) & dmasks[n])); } - *p = tmp; - (*p)++; + *p++ = tmp; } /* write the middle words */