12:20 p.m.
To test this change I experimentally modified the file plugin to pass
NULL to nbdkit_realpath, and at least in this simple explicit case
both GCC and Clang give warnings (which turn into errors when using
‘./configure --enable-gcc-warnings’).
GCC warns:
file.c: In function ‘file_config’:
file.c:86:16: error: null argument where non-null required (argument 1) [-Werror=nonnull]
filename = nbdkit_realpath (NULL);
^~~~~~~~~~~~~~~
Clang warns:
file.c:86:37: error: null passed to a callee that requires a non-null argument
[-Werror,-Wnonnull]
filename = nbdkit_realpath (NULL);
~~~~^
Libvirt has an interesting history with this attribute. In 2012 use
of the attribute was disabled when compiling under GCC (but kept for
Coverity) because of poor behaviour of GCC. This is still the case
for libvirt upstream. However the bug in GCC does appear to have been
fixed at the end of 2016.
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=eefb881d4683d50882b...
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308
---
common/bitmap/bitmap.h | 18 ++++++++++--------
common/include/iszero.h | 2 +-
common/include/nextnonzero.h | 2 +-
common/include/random.h | 4 ++--
common/regions/regions.h | 19 ++++++++++++-------
common/sparse/sparse.h | 12 +++++++++---
include/nbdkit-common.h | 15 ++++++++++-----
common/sparse/sparse.c | 10 ++++++----
server/utils.c | 8 ++++----
9 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/common/bitmap/bitmap.h b/common/bitmap/bitmap.h
index 0da650a..553b9d2 100644
--- a/common/bitmap/bitmap.h
+++ b/common/bitmap/bitmap.h
@@ -65,7 +65,7 @@ struct bitmap {
size_t size; /* Size of bitmap in bytes. */
};
-static inline void
+static inline void __attribute__((__nonnull__ (1)))
bitmap_init (struct bitmap *bm, unsigned blksize, unsigned bpb)
{
assert (is_power_of_2 (blksize));
@@ -99,10 +99,11 @@ bitmap_free (struct bitmap *bm)
/* Resize the bitmap to the virtual disk size in bytes.
* Returns -1 on error, setting nbdkit_error.
*/
-extern int bitmap_resize (struct bitmap *bm, uint64_t new_size);
+extern int bitmap_resize (struct bitmap *bm, uint64_t new_size)
+ __attribute__((__nonnull__ (1)));
/* Clear the bitmap (set everything to zero). */
-static inline void
+static inline void __attribute__((__nonnull__ (1)))
bitmap_clear (struct bitmap *bm)
{
memset (bm->bitmap, 0, bm->size);
@@ -125,7 +126,7 @@ bitmap_clear (struct bitmap *bm)
/* Return the bit(s) associated with the given block.
* If the request is out of range, returns the default value.
*/
-static inline unsigned
+static inline unsigned __attribute__((__nonnull__ (1)))
bitmap_get_blk (const struct bitmap *bm, uint64_t blk, unsigned default_)
{
BITMAP_OFFSET_BIT_MASK (bm, blk);
@@ -139,7 +140,7 @@ bitmap_get_blk (const struct bitmap *bm, uint64_t blk, unsigned
default_)
}
/* As above but works with virtual disk offset in bytes. */
-static inline unsigned
+static inline unsigned __attribute__((__nonnull__ (1)))
bitmap_get (const struct bitmap *bm, uint64_t offset, unsigned default_)
{
return bitmap_get_blk (bm, offset / bm->blksize, default_);
@@ -148,7 +149,7 @@ bitmap_get (const struct bitmap *bm, uint64_t offset, unsigned
default_)
/* Set the bit(s) associated with the given block.
* If out of range, it is ignored.
*/
-static inline void
+static inline void __attribute__((__nonnull__ (1)))
bitmap_set_blk (const struct bitmap *bm, uint64_t blk, unsigned v)
{
BITMAP_OFFSET_BIT_MASK (bm, blk);
@@ -163,7 +164,7 @@ bitmap_set_blk (const struct bitmap *bm, uint64_t blk, unsigned v)
}
/* As above bit works with virtual disk offset in bytes. */
-static inline void
+static inline void __attribute__((__nonnull__ (1)))
bitmap_set (const struct bitmap *bm, uint64_t offset, unsigned v)
{
return bitmap_set_blk (bm, offset / bm->blksize, v);
@@ -177,6 +178,7 @@ bitmap_set (const struct bitmap *bm, uint64_t offset, unsigned v)
* Returns -1 if the bitmap is all zeroes from blk to the end of the
* bitmap.
*/
-extern int64_t bitmap_next (const struct bitmap *bm, uint64_t blk);
+extern int64_t bitmap_next (const struct bitmap *bm, uint64_t blk)
+ __attribute__((__nonnull__ (1)));
#endif /* NBDKIT_BITMAP_H */
diff --git a/common/include/iszero.h b/common/include/iszero.h
index 7a54f0a..efe0762 100644
--- a/common/include/iszero.h
+++ b/common/include/iszero.h
@@ -46,7 +46,7 @@
* See also:
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69908
*/
-static inline bool
+static inline bool __attribute__((__nonnull__ (1)))
is_zero (const char *buffer, size_t size)
{
size_t i;
diff --git a/common/include/nextnonzero.h b/common/include/nextnonzero.h
index 3f96e85..f25000b 100644
--- a/common/include/nextnonzero.h
+++ b/common/include/nextnonzero.h
@@ -45,7 +45,7 @@
* https://sourceware.org/bugzilla/show_bug.cgi?id=19920
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69908
*/
-static inline const char *
+static inline const char * __attribute__((__nonnull__ (1)))
next_non_zero (const char *buffer, size_t size)
{
size_t i;
diff --git a/common/include/random.h b/common/include/random.h
index eb08295..098c6d6 100644
--- a/common/include/random.h
+++ b/common/include/random.h
@@ -67,7 +67,7 @@ snext (uint64_t *seed)
}
/* Seed the random state from a 64 bit seed. */
-static inline void
+static inline void __attribute__((__nonnull__ (2)))
xsrandom (uint64_t seed, struct random_state *state)
{
state->s[0] = snext (&seed);
@@ -83,7 +83,7 @@ rotl (const uint64_t x, int k)
}
/* Returns 64 random bits. Updates the state. */
-static inline uint64_t
+static inline uint64_t __attribute__((__nonnull__ (1)))
xrandom (struct random_state *state)
{
const uint64_t result_starstar = rotl (state->s[1] * 5, 7) * 9;
diff --git a/common/regions/regions.h b/common/regions/regions.h
index ca9b3d5..4fcaf09 100644
--- a/common/regions/regions.h
+++ b/common/regions/regions.h
@@ -76,25 +76,30 @@ struct regions {
size_t nr_regions;
};
-extern void init_regions (struct regions *regions);
-extern void free_regions (struct regions *regions);
-extern const struct region *find_region (const struct regions *regions, uint64_t
offset);
-extern int append_region (struct regions *regions, struct region region);
+extern void init_regions (struct regions *regions)
+ __attribute__((__nonnull__ (1)));
+extern void free_regions (struct regions *regions)
+ __attribute__((__nonnull__ (1)));
+extern const struct region *find_region (const struct regions *regions,
+ uint64_t offset)
+ __attribute__((__nonnull__ (1)));
+extern int append_region (struct regions *regions, struct region region)
+ __attribute__((__nonnull__ (1)));
-static inline const struct region *
+static inline const struct region * __attribute__((__nonnull__ (1)))
get_region (const struct regions *regions, size_t i)
{
assert (i < regions->nr_regions);
return ®ions->regions[i];
}
-static inline size_t
+static inline size_t __attribute__((__nonnull__ (1)))
nr_regions (struct regions *regions)
{
return regions->nr_regions;
}
-static inline int64_t
+static inline int64_t __attribute__((__nonnull__ (1)))
virtual_size (struct regions *regions)
{
if (regions->nr_regions == 0)
diff --git a/common/sparse/sparse.h b/common/sparse/sparse.h
index 3acb0af..818d804 100644
--- a/common/sparse/sparse.h
+++ b/common/sparse/sparse.h
@@ -64,12 +64,16 @@ extern void free_sparse_array (struct sparse_array *sa);
/* Read bytes from the sparse array.
* Note this can never return an error and never allocates.
*/
-extern void sparse_array_read (struct sparse_array *sa, void *buf, uint32_t count,
uint64_t offset);
+extern void sparse_array_read (struct sparse_array *sa, void *buf,
+ uint32_t count, uint64_t offset)
+ __attribute__((__nonnull__ (1, 2)));
/* Write bytes to the sparse array.
* This can allocate and can return an error.
*/
-extern int sparse_array_write (struct sparse_array *sa, const void *buf, uint32_t count,
uint64_t offset);
+extern int sparse_array_write (struct sparse_array *sa, const void *buf,
+ uint32_t count, uint64_t offset)
+ __attribute__((__nonnull__ (1, 2)));
/* Zero byte range in the sparse array.
*
@@ -79,6 +83,8 @@ extern int sparse_array_write (struct sparse_array *sa, const void *buf,
uint32_
*
* This may free memory, but never returns an error.
*/
-extern void sparse_array_zero (struct sparse_array *sa, uint32_t count, uint64_t
offset);
+extern void sparse_array_zero (struct sparse_array *sa,
+ uint32_t count, uint64_t offset)
+ __attribute__((__nonnull__ (1)));
#endif /* NBDKIT_SPARSE_H */
diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index 27f6ed1..e58ca8a 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -64,11 +64,16 @@ extern void nbdkit_debug (const char *msg, ...)
__attribute__((__format__ (__printf__, 1, 2)));
extern void nbdkit_vdebug (const char *msg, va_list args);
-extern char *nbdkit_absolute_path (const char *path);
-extern int64_t nbdkit_parse_size (const char *str);
-extern int nbdkit_parse_bool (const char *str);
-extern int nbdkit_read_password (const char *value, char **password);
-extern char *nbdkit_realpath (const char *path);
+extern char *nbdkit_absolute_path (const char *path)
+ __attribute__((__nonnull__ (1)));
+extern int64_t nbdkit_parse_size (const char *str)
+ __attribute__((__nonnull__ (1)));
+extern int nbdkit_parse_bool (const char *str)
+ __attribute__((__nonnull__ (1)));
+extern int nbdkit_read_password (const char *value, char **password)
+ __attribute__((__nonnull__ (1, 2)));
+extern char *nbdkit_realpath (const char *path)
+ __attribute__((__nonnull__ (1)));
/* A static non-NULL pointer which can be used when you don't need a
* per-connection handle.
diff --git a/common/sparse/sparse.c b/common/sparse/sparse.c
index be4eb2c..a5ace48 100644
--- a/common/sparse/sparse.c
+++ b/common/sparse/sparse.c
@@ -123,10 +123,12 @@ free_sparse_array (struct sparse_array *sa)
{
size_t i;
- for (i = 0; i < sa->l1_size; ++i)
- free_l2_dir (sa->l1_dir[i].l2_dir);
- free (sa->l1_dir);
- free (sa);
+ if (sa) {
+ for (i = 0; i < sa->l1_size; ++i)
+ free_l2_dir (sa->l1_dir[i].l2_dir);
+ free (sa->l1_dir);
+ free (sa);
+ }
}
struct sparse_array *
diff --git a/server/utils.c b/server/utils.c
index 18011fd..ceacb39 100644
--- a/server/utils.c
+++ b/server/utils.c
@@ -54,8 +54,8 @@ nbdkit_absolute_path (const char *path)
CLEANUP_FREE char *pwd = NULL;
char *ret;
- if (path == NULL || *path == '\0') {
- nbdkit_error ("cannot convert null or empty path to an absolute path");
+ if (*path == '\0') {
+ nbdkit_error ("cannot convert empty path to an absolute path");
return NULL;
}
@@ -261,8 +261,8 @@ nbdkit_realpath (const char *path)
{
char *ret;
- if (path == NULL || *path == '\0') {
- nbdkit_error ("cannot resolve a null or empty path");
+ if (*path == '\0') {
+ nbdkit_error ("realpath: cannot resolve an empty path");
return NULL;
}
--
2.19.2