9:30 a.m.
Follow-up to this patch:
https://listman.redhat.com/archives/libguestfs/2021-November/msg00251.html
Patches 1-3 are probably down to over-active warnings in the old
version of GCC. Patch 3 in particular is bogus, but needed to get
around the warning.
Patch 4 moves the test so it's next to the header file being tested.
I don't mind if you squash these into your patch before pushing.
Rich.
9:30 a.m.
New subject: [PATCH nbdkit v2 follow-up 1/4] Add parentheses where suggested by RHEL 7 GCC 4.8.5
In file included from vector.c:39:0:
../../common/include/checked-overflow.h: In function 'check_mul_overflow':
../../common/include/checked-overflow.h:191:45: error: suggest parentheses around
'&&' within '||' [-Werror=parentheses]
in_range = b == 0 || a <= UINTMAX_MAX / b && *r <= max;
^
---
common/include/checked-overflow.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/include/checked-overflow.h b/common/include/checked-overflow.h
index 7f520036..d72f95a2 100644
--- a/common/include/checked-overflow.h
+++ b/common/include/checked-overflow.h
@@ -188,7 +188,7 @@ check_mul_overflow (uintmax_t a, uintmax_t b, uintmax_t max, uintmax_t
*r)
bool in_range;
*r = a * b;
- in_range = b == 0 || a <= UINTMAX_MAX / b && *r <= max;
+ in_range = b == 0 || (a <= UINTMAX_MAX / b && *r <= max);
return !in_range;
}
--
2.32.0
9:30 a.m.
New subject: [PATCH nbdkit v2 follow-up 2/4] Add __attribute__((__unused__))
../../common/include/checked-overflow.h:164:18: error: typedef 'x_has_uint_type'
locally defined but not used [-Werror=unused-local-typedefs]
typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
^
---
common/include/checked-overflow.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/include/checked-overflow.h b/common/include/checked-overflow.h
index d72f95a2..01943948 100644
--- a/common/include/checked-overflow.h
+++ b/common/include/checked-overflow.h
@@ -161,7 +161,7 @@
*/
#define STATIC_ASSERT_UNSIGNED_INT(x) \
do { \
- typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
+ typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]
__attribute__((__unused__)); \
} while (0)
/* Assign the sum "a + b" to "*r", using uintmax_t modular
arithmetic.
--
2.32.0
9:55 a.m.
New subject: [PATCH nbdkit v2 follow-up 2/4] Add __attribute__((__unused__))
On 11/25/21 16:30, Richard W.M. Jones wrote:
../../common/include/checked-overflow.h:164:18: error: typedef
'x_has_uint_type' locally defined but not used [-Werror=unused-local-typedefs]
typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
^
---
common/include/checked-overflow.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/include/checked-overflow.h b/common/include/checked-overflow.h
index d72f95a2..01943948 100644
--- a/common/include/checked-overflow.h
+++ b/common/include/checked-overflow.h
@@ -161,7 +161,7 @@
*/
#define STATIC_ASSERT_UNSIGNED_INT(x) \
do { \
- typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
+ typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]
__attribute__((__unused__)); \
} while (0)
/* Assign the sum "a + b" to "*r", using uintmax_t modular
arithmetic.
This breaks the nice 80 chars width; can I move
__attribute__((__unused__)) to the next line? If so, how many space
chars should I use for indentation, relative to the "typedef"?
Thanks!
Laszlo
10:01 a.m.
New subject: [PATCH nbdkit v2 follow-up 2/4] Add __attribute__((__unused__))
On Thu, Nov 25, 2021 at 04:55:25PM +0100, Laszlo Ersek wrote:
On 11/25/21 16:30, Richard W.M. Jones wrote:
> ../../common/include/checked-overflow.h:164:18: error: typedef
'x_has_uint_type' locally defined but not used [-Werror=unused-local-typedefs]
> typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
> ^
> ---
> common/include/checked-overflow.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/include/checked-overflow.h b/common/include/checked-overflow.h
> index d72f95a2..01943948 100644
> --- a/common/include/checked-overflow.h
> +++ b/common/include/checked-overflow.h
> @@ -161,7 +161,7 @@
> */
> #define STATIC_ASSERT_UNSIGNED_INT(x) \
> do { \
> - typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
> + typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]
__attribute__((__unused__)); \
> } while (0)
>
> /* Assign the sum "a + b" to "*r", using uintmax_t modular
arithmetic.
>
This breaks the nice 80 chars width; can I move
__attribute__((__unused__)) to the next line? If so, how many space
chars should I use for indentation, relative to the "typedef"?
Sure; no idea!
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
3:36 a.m.
New subject: [PATCH nbdkit v2 follow-up 2/4] Add __attribute__((__unused__))
On 11/25/21 17:01, Richard W.M. Jones wrote:
On Thu, Nov 25, 2021 at 04:55:25PM +0100, Laszlo Ersek wrote:
> On 11/25/21 16:30, Richard W.M. Jones wrote:
>> ../../common/include/checked-overflow.h:164:18: error: typedef
'x_has_uint_type' locally defined but not used [-Werror=unused-local-typedefs]
>> typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
>> ^
>> ---
>> common/include/checked-overflow.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/include/checked-overflow.h
b/common/include/checked-overflow.h
>> index d72f95a2..01943948 100644
>> --- a/common/include/checked-overflow.h
>> +++ b/common/include/checked-overflow.h
>> @@ -161,7 +161,7 @@
>> */
>> #define STATIC_ASSERT_UNSIGNED_INT(x) \
>> do { \
>> - typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]; \
>> + typedef char x_has_uint_type[(typeof (x))-1 > 0 ? 1 : -1]
__attribute__((__unused__)); \
>> } while (0)
>>
>> /* Assign the sum "a + b" to "*r", using uintmax_t modular
arithmetic.
>>
>
> This breaks the nice 80 chars width; can I move
> __attribute__((__unused__)) to the next line? If so, how many space
> chars should I use for indentation, relative to the "typedef"?
Sure; no idea!
Thanks, and interestingly, "prior art" is not consistent:
- In "common/allocators/allocator.h", we have __attribute__ aligned with
the very left of the declaration on the previous line (multiple instances).
- One instance in the same header, and then several in
"common/bitmap/bitmap.h", "common/regions/regions.h",
"common/replacements/syslog.h", indent __attribute__ two spaces relative
to the start of the declaration.
I'll use two spaces for indentation.
(BTW I meant to build this on RHEL7 myself, but then the amount of build
dependencies likely needed discouraged me...)
Thanks!
Laszlo
9:30 a.m.
New subject: [PATCH nbdkit v2 follow-up 3/4] Avoid shadowed declaration
In file included from test-checked-overflow.c:38:0:
../../common/include/checked-overflow.h:146:10: error: declaration of ‘overflow’ shadows a
previous local [-Werror=shadow]
bool overflow; \
^
---
common/utils/test-checked-overflow.c | 46 ++++++++++++++--------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/common/utils/test-checked-overflow.c b/common/utils/test-checked-overflow.c
index c2882e38..cfbcb4f2 100644
--- a/common/utils/test-checked-overflow.c
+++ b/common/utils/test-checked-overflow.c
@@ -39,25 +39,25 @@
#define TEST_ADD(a, b, result, expected_overflow, expected_result) \
do { \
- bool overflow; \
+ bool overflow0; \
\
- overflow = ADD_OVERFLOW_FALLBACK ((a), (b), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow0 = ADD_OVERFLOW_FALLBACK ((a), (b), (result)); \
+ assert (overflow0 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
- overflow = ADD_OVERFLOW_FALLBACK ((b), (a), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow0 = ADD_OVERFLOW_FALLBACK ((b), (a), (result)); \
+ assert (overflow0 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
} while (0)
#define TEST_MUL(a, b, result, expected_overflow, expected_result) \
do { \
- bool overflow; \
+ bool overflow1; \
\
- overflow = MUL_OVERFLOW_FALLBACK ((a), (b), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow1 = MUL_OVERFLOW_FALLBACK ((a), (b), (result)); \
+ assert (overflow1 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
- overflow = MUL_OVERFLOW_FALLBACK ((b), (a), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow1 = MUL_OVERFLOW_FALLBACK ((b), (a), (result)); \
+ assert (overflow1 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
} while (0)
@@ -84,7 +84,7 @@ main (void)
uint8_t u8;
size_t sz;
} result;
- bool overflow;
+ bool overflow2;
/* "max + 0" and "0 + max" evaluate to "max", without
overflow. */
TEST_ADD (umax_max, 0u, &result.umax, false, umax_max);
@@ -151,26 +151,26 @@ main (void)
*
* Perform the above multiplications, advacing with prime factors.
*/
- overflow = MUL_OVERFLOW_FALLBACK (3u, 5u, &result.u8);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (3u, 5u, &result.u8);
+ assert (!overflow2);
assert (result.u8 == 0xF);
- overflow = MUL_OVERFLOW_FALLBACK (result.u8, 17u, &result.u8);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u8, 17u, &result.u8);
+ assert (!overflow2);
assert (result.u8 == UINT8_MAX);
- overflow = MUL_OVERFLOW_FALLBACK (result.u8, 257u, &result.u16);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u8, 257u, &result.u16);
+ assert (!overflow2);
assert (result.u16 == UINT16_MAX);
- overflow = MUL_OVERFLOW_FALLBACK (result.u16, 65537ul, &result.u32);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u16, 65537ul, &result.u32);
+ assert (!overflow2);
assert (result.u32 == UINT32_MAX);
- overflow = MUL_OVERFLOW_FALLBACK (result.u32, 641u, &result.u64);
- assert (!overflow);
- overflow = MUL_OVERFLOW_FALLBACK (result.u64, 6700417ul, &result.u64);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u32, 641u, &result.u64);
+ assert (!overflow2);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u64, 6700417ul, &result.u64);
+ assert (!overflow2);
assert (result.u64 == UINT64_MAX);
return 0;
--
2.32.0
4:08 a.m.
New subject: [PATCH nbdkit v2 follow-up 3/4] Avoid shadowed declaration
On 11/25/21 16:30, Richard W.M. Jones wrote:
In file included from test-checked-overflow.c:38:0:
../../common/include/checked-overflow.h:146:10: error: declaration of ‘overflow’ shadows
a previous local [-Werror=shadow]
bool overflow; \
^
---
common/utils/test-checked-overflow.c | 46 ++++++++++++++--------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/common/utils/test-checked-overflow.c b/common/utils/test-checked-overflow.c
index c2882e38..cfbcb4f2 100644
--- a/common/utils/test-checked-overflow.c
+++ b/common/utils/test-checked-overflow.c
@@ -39,25 +39,25 @@
#define TEST_ADD(a, b, result, expected_overflow, expected_result) \
do { \
- bool overflow; \
+ bool overflow0; \
\
- overflow = ADD_OVERFLOW_FALLBACK ((a), (b), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow0 = ADD_OVERFLOW_FALLBACK ((a), (b), (result)); \
+ assert (overflow0 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
- overflow = ADD_OVERFLOW_FALLBACK ((b), (a), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow0 = ADD_OVERFLOW_FALLBACK ((b), (a), (result)); \
+ assert (overflow0 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
} while (0)
#define TEST_MUL(a, b, result, expected_overflow, expected_result) \
do { \
- bool overflow; \
+ bool overflow1; \
\
- overflow = MUL_OVERFLOW_FALLBACK ((a), (b), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow1 = MUL_OVERFLOW_FALLBACK ((a), (b), (result)); \
+ assert (overflow1 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
- overflow = MUL_OVERFLOW_FALLBACK ((b), (a), (result)); \
- assert (overflow == (expected_overflow)); \
+ overflow1 = MUL_OVERFLOW_FALLBACK ((b), (a), (result)); \
+ assert (overflow1 == (expected_overflow)); \
assert (*(result) == (expected_result)); \
} while (0)
@@ -84,7 +84,7 @@ main (void)
uint8_t u8;
size_t sz;
} result;
- bool overflow;
+ bool overflow2;
/* "max + 0" and "0 + max" evaluate to "max", without
overflow. */
TEST_ADD (umax_max, 0u, &result.umax, false, umax_max);
@@ -151,26 +151,26 @@ main (void)
*
* Perform the above multiplications, advacing with prime factors.
*/
- overflow = MUL_OVERFLOW_FALLBACK (3u, 5u, &result.u8);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (3u, 5u, &result.u8);
+ assert (!overflow2);
assert (result.u8 == 0xF);
- overflow = MUL_OVERFLOW_FALLBACK (result.u8, 17u, &result.u8);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u8, 17u, &result.u8);
+ assert (!overflow2);
assert (result.u8 == UINT8_MAX);
- overflow = MUL_OVERFLOW_FALLBACK (result.u8, 257u, &result.u16);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u8, 257u, &result.u16);
+ assert (!overflow2);
assert (result.u16 == UINT16_MAX);
- overflow = MUL_OVERFLOW_FALLBACK (result.u16, 65537ul, &result.u32);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u16, 65537ul, &result.u32);
+ assert (!overflow2);
assert (result.u32 == UINT32_MAX);
- overflow = MUL_OVERFLOW_FALLBACK (result.u32, 641u, &result.u64);
- assert (!overflow);
- overflow = MUL_OVERFLOW_FALLBACK (result.u64, 6700417ul, &result.u64);
- assert (!overflow);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u32, 641u, &result.u64);
+ assert (!overflow2);
+ overflow2 = MUL_OVERFLOW_FALLBACK (result.u64, 6700417ul, &result.u64);
+ assert (!overflow2);
assert (result.u64 == UINT64_MAX);
return 0;
We already have UNIQUE_VAR() :) I'll try to use that, instead.
Thanks!
Laszlo
9:30 a.m.
New subject: [PATCH nbdkit v2 follow-up 4/4] Move test binary to common/include
So it is next to the header file which it tests.
---
common/include/Makefile.am | 5 +++++
common/utils/Makefile.am | 8 ++------
common/{utils => include}/test-checked-overflow.c | 0
.gitignore | 1 +
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/common/include/Makefile.am b/common/include/Makefile.am
index 52d97216..b1da8f65 100644
--- a/common/include/Makefile.am
+++ b/common/include/Makefile.am
@@ -56,6 +56,7 @@ TESTS = \
test-ascii-ctype \
test-ascii-string \
test-byte-swapping \
+ test-checked-overflow \
test-isaligned \
test-ispowerof2 \
test-iszero \
@@ -78,6 +79,10 @@ test_byte_swapping_SOURCES = test-byte-swapping.c byte-swapping.h
test_byte_swapping_CPPFLAGS = -I$(srcdir)
test_byte_swapping_CFLAGS = $(WARNINGS_CFLAGS)
+test_checked_overflow_SOURCES = test-checked-overflow.c
+test_checked_overflow_CPPFLAGS = -I$(top_srcdir)/common/include
+test_checked_overflow_CFLAGS = $(WARNINGS_CFLAGS)
+
test_isaligned_SOURCES = test-isaligned.c isaligned.h
test_isaligned_CPPFLAGS = -I$(srcdir)
test_isaligned_CFLAGS = $(WARNINGS_CFLAGS)
diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
index 5218a699..012a5c25 100644
--- a/common/utils/Makefile.am
+++ b/common/utils/Makefile.am
@@ -94,12 +94,8 @@ windows-errors.c: windows-errors.txt
# Unit tests.
-TESTS = test-checked-overflow test-quotes test-vector
-check_PROGRAMS = test-checked-overflow test-quotes test-vector
-
-test_checked_overflow_SOURCES = test-checked-overflow.c
-test_checked_overflow_CPPFLAGS = -I$(top_srcdir)/common/include
-test_checked_overflow_CFLAGS = $(WARNINGS_CFLAGS)
+TESTS = test-quotes test-vector
+check_PROGRAMS = test-quotes test-vector
test_quotes_SOURCES = test-quotes.c quote.c utils.h
test_quotes_CPPFLAGS = -I$(srcdir)
diff --git a/common/utils/test-checked-overflow.c
b/common/include/test-checked-overflow.c
similarity index 100%
rename from common/utils/test-checked-overflow.c
rename to common/include/test-checked-overflow.c
diff --git a/.gitignore b/.gitignore
index 4e2ae75d..2d27cf06 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,6 +37,7 @@ plugins/*/*.3
/common/include/test-ascii-ctype
/common/include/test-ascii-string
/common/include/test-byte-swapping
+/common/include/test-checked-overflow
/common/include/test-isaligned
/common/include/test-ispowerof2
/common/include/test-iszero
--
2.32.0
1091
days inactive
1092
days old
8 comments
2 participants
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones