1:51 a.m.
From: Eric Blake <eblake(a)redhat.com>
Original commit message:
I ran:
$ cppcheck --cppcheck-build-dir=cache/cppcheck -q -I . -I common/include/ \
-I include/ -I lib/ -I common/utils/ -i generator/ .
then inspected the output. cppcheck correctly flags:
copy/file-ops.c:423:10: portability: 'data' is of type 'void *'. When
using void pointers in calculations, the behaviour is undefined. Arithmetic operations on
'void *' is a GNU C extension, which defines the 'sizeof(void)' to be 1.
[arithOperationsOnVoidPointer]
examples/copy-libev.c:312:10: style: Local variable 'is_zero' shadows outer
function [shadowFunction]
ocaml/nbd-c.c:419:16: portability: Shifting signed 32-bit value by 31 bits is
implementation-defined behaviour. See condition at line 416. [shiftTooManyBitsSigned]
It also has several easy-to-fix false positives such as:
lib/opt.c:98:17: error: Uninitialized variable: err [uninitvar]
where the tool failed to see that err was always assigned by the
callback function, or
python/methods.c:357:7: style: Unused variable: ret [unusedVariable]
where the tool prefers redundant ; to delimit use of Python macros
from actual assignments. I even addressed some of its style
complaints, like:
copy/nbd-ops.c:466:16: style: The scope of the variable 'd' can be reduced...
examples/open-qcow2.c:12:23: style: Parameter 'argv' can be declared with const
[constParameter]
common/utils/test-vector.c:53:0: style: The function 'int64_vector_append' is
never used. [unusedFunction]
Then there are other false positives that are harder to silence (for
example, it complains about the macros ARRAY_SIZE(), ADD_OVERFLOW(),
MUL_OVERFLOW(); incorrectly suggesting to add const to callback
function signatures; ...) which I don't try to address here. So at
this point, it's not yet worth automating a run of cppcheck into our
CI.
Porting notes:
- Only pick the "common/utils/vector.h" and
"common/utils/test-vector.c"
code changes, for bringing nbdkit's common/ subdir closer to that of
libnbd.
- Extend "__attribute__((__unused__))" to name##_duplicate(). This
function comes from nbdkit commit 78b72746e547 ("vector: Add
vector_duplicate function", 2021-08-22), and was not present in libnbd
at the time of libnbd commit 8fb8ffb53477 ("build: Silence some cppcheck
warnings", 2022-11-02).
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
(cherry picked from libnbd commit 8fb8ffb534774e4fc9115b484277713285238078)
---
common/utils/vector.h | 20 ++++++++++----------
common/utils/test-vector.c | 3 +--
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/common/utils/vector.h b/common/utils/vector.h
index 347a85735e81..d3a669251fa8 100644
--- a/common/utils/vector.h
+++ b/common/utils/vector.h
@@ -94,7 +94,7 @@
* allocated and capacity is increased, but the vector length is \
* not increased and the new elements are not initialized. \
*/ \
- static inline int \
+ static inline int __attribute__((__unused__)) \
name##_reserve (name *v, size_t n) \
{ \
return generic_vector_reserve ((struct generic_vector *)v, n, \
@@ -104,7 +104,7 @@
/* Same as _reserve, but the allocation will be page aligned. Note \
* that the machine page size must be divisible by sizeof (type). \
*/ \
- static inline int \
+ static inline int __attribute__((__unused__)) \
name##_reserve_page_aligned (name *v, size_t n) \
{ \
return generic_vector_reserve_page_aligned ((struct generic_vector *)v, \
@@ -112,7 +112,7 @@
} \
\
/* Insert at i'th element. i=0 => beginning i=len => append */ \
- static inline int \
+ static inline int __attribute__((__unused__)) \
name##_insert (name *v, type elem, size_t i) \
{ \
assert (i <= v->len); \
@@ -126,14 +126,14 @@
} \
\
/* Append a new element to the end of the vector. */ \
- static inline int \
+ static inline int __attribute__((__unused__)) \
name##_append (name *v, type elem) \
{ \
return name##_insert (v, elem, v->len); \
} \
\
/* Remove i'th element. i=0 => beginning i=len-1 => end */ \
- static inline void \
+ static inline void __attribute__((__unused__)) \
name##_remove (name *v, size_t i) \
{ \
assert (i < v->len); \
@@ -142,7 +142,7 @@
} \
\
/* Remove all elements and deallocate the vector. */ \
- static inline void \
+ static inline void __attribute__((__unused__)) \
name##_reset (name *v) \
{ \
free (v->ptr); \
@@ -151,7 +151,7 @@
} \
\
/* Iterate over the vector, calling f() on each element. */ \
- static inline void \
+ static inline void __attribute__((__unused__)) \
name##_iter (name *v, void (*f) (type elem)) \
{ \
size_t i; \
@@ -160,7 +160,7 @@
} \
\
/* Sort the elements of the vector. */ \
- static inline void \
+ static inline void __attribute__((__unused__)) \
name##_sort (name *v, \
int (*compare) (const type *p1, const type *p2)) \
{ \
@@ -170,7 +170,7 @@
/* Search for an exactly matching element in the vector using a \
* binary search. Returns a pointer to the element or NULL. \
*/ \
- static inline type * \
+ static inline type * __attribute__((__unused__)) \
name##_search (const name *v, const void *key, \
int (*compare) (const void *key, const type *v)) \
{ \
@@ -179,7 +179,7 @@
} \
\
/* Make a new vector with the same elements. */ \
- static inline int \
+ static inline int __attribute__((__unused__)) \
name##_duplicate (name *v, name *copy) \
{ \
/* Note it's allowed for v and copy to be the same pointer. */ \
diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
index 19b826763f04..5249acd594f6 100644
--- a/common/utils/test-vector.c
+++ b/common/utils/test-vector.c
@@ -64,11 +64,10 @@ test_int64_vector (void)
{
int64_vector v = empty_vector;
size_t i;
- int r;
int64_t tmp, *p;
for (i = 0; i < 10; ++i) {
- r = int64_vector_insert (&v, i, 0);
+ int r = int64_vector_insert (&v, i, 0);
assert (r == 0);
}