3:58 p.m.
On 1/2/19 2:14 PM, Richard W.M. Jones wrote:
Annotate some function parameters with attribute((nonnull)). Only
do
this for internal headers where we are sure that we will be using
sufficiently recent GCC or Clang. For the public header files
(ie. include/nbdkit-*.h) it may be that people building out of tree
plugins are using old GCC which had problems, or even other compilers
that don't support this extension at all.
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
---
+++ b/filters/cache/blk.h
This file is not on the current nbdkit.git master; I'm assuming the
patch depends on some of your other patches landing first.
@@ -51,10 +51,15 @@ extern void blk_free (void);
extern int blk_set_size (uint64_t new_size);
/* Read a single block from the cache or plugin. */
-extern int blk_read (struct nbdkit_next_ops *next_ops, void *nxdata, uint64_t blknum,
uint8_t *block, int *err);
+extern int blk_read (struct nbdkit_next_ops *next_ops, void *nxdata,
+ uint64_t blknum, uint8_t *block, int *err)
+ __attribute__((__nonnull__ (1, 2, 4, 5)));
nxdata is opaque - which means blk_read() shouldn't be dereferencing it.
It may happen to be true that struct backend is set up such that nxdata
is currently always non-NULL, but that's an implementation detail, and
we are violating layering if we enforce backend behavior by insisting on
the opaque parameter having or not having any particular value, rather
than just blindly passing it on through and letting the backend do the
enforcing. I would use just __nonnull__ (1, 4, 5).
/* Write to the cache and the plugin. */
-extern int blk_writethrough (struct nbdkit_next_ops *next_ops, void *nxdata, uint64_t
blknum, const uint8_t *block, uint32_t flags, int *err);
+extern int blk_writethrough (struct nbdkit_next_ops *next_ops, void *nxdata,
+ uint64_t blknum, const uint8_t *block,
+ uint32_t flags, int *err)
+ __attribute__((__nonnull__ (1, 2, 4, 6)));
Same comment; probably repeats elsewhere in the patch.
+++ b/server/internal.h
+extern pthread_mutex_t *connection_get_request_lock (struct
connection *conn)
+ __attribute__((__nonnull__ (1)));
+extern void connection_set_crypto_session (struct connection *conn,
+ void *session)
+ __attribute__((__nonnull__ (1 /* not 2 */)));
+extern void *connection_get_crypto_session (struct connection *conn)
+ __attribute__((__nonnull__ (1 /* not 2 */)));
Spurious comment here (but correct use of not decorating parameters in
earlier functions that are opaque).
+++ b/tests/test.h
@@ -44,7 +44,8 @@ extern const char *sock; /* socket of most recent nbdkit process
*/
extern const char *server[2]; /* server parameter for add_drive */
/* Can be called more than once (useful for nbd plugin) */
-extern int test_start_nbdkit (const char *arg, ...);
+extern int test_start_nbdkit (const char *arg, ...)
+ __attribute__((__nonnull__ (1)));
Independent fix: this should probably also have
__attribute__((__sentinel__))
All the other changes look sane.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org