9:04 a.m.
On Fri, Feb 24, 2023 at 05:39:35AM +0100, Laszlo Ersek wrote:
A semicolon after a DEFINE_VECTOR_TYPE(...) macro invocation is not
needed, nor does our documentation in "common/utils/vector.h" prescribe
one. Furthermore, it breaks ISO C, which gcc reports with "-Wpedantic":
> warning: ISO C does not allow extra ‘;’ outside of a function
> [-Wpedantic]
Remove the semicolons. (Note that we have such DEFINE_VECTOR_TYPE()
invocations too that are not followed by a semicolon, so our current usage
is inconsistently wrong.)
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
Notes:
v4:
- new patch in v4
lib/internal.h | 2 +-
common/utils/const-string-vector.h | 2 +-
common/utils/nbdkit-string.h | 2 +-
common/utils/string-vector.h | 2 +-
common/utils/test-vector.c | 4 ++--
copy/nbdcopy.h | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 0b5f793011b8..b88b43ec6e6b 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -71,7 +71,7 @@ struct meta_context {
char *name; /* Name of meta context. */
uint32_t context_id; /* Context ID negotiated with the server. */
};
-DEFINE_VECTOR_TYPE (meta_vector, struct meta_context);
+DEFINE_VECTOR_TYPE (meta_vector, struct meta_context)
struct export {
Emacs (and probably other editors with auto-indent features) has a
tough time with this style choice. I agree that because the macro
ends with a function definition for name##_search(){}, neither
including a trailing semicolon in the macro nor supplying one after
expanding the macro is syntactically necessary. But without the
semicolon after the invocation, emacs thinks that DEFINE_VECTOR_TYPE()
starts an incomplete declaration, and hitting <TAB> on the next text
('struct export {') causes indentation by two more spaces as a
continuation.
I agree that our code base is inconsistent on whether we supply or
omit a trailing semicolon, but I would also like to appease the editor
syntax engine. What if we instead go for a consistent style in the
opposite direction? My counter-proposal: hack the definition of the
macro to REQUIRE that callers include a trailing semicolon. Here's
the easiest way I could think of:
diff --git c/common/utils/vector.h i/common/utils/vector.h
index fb2482c8..00b6a0da 100644
--- c/common/utils/vector.h
+++ i/common/utils/vector.h
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2020-2022 Red Hat Inc.
+ * Copyright (C) 2020-2023 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -176,7 +176,10 @@
{ \
return bsearch (key, v->ptr, v->len, sizeof (type), \
(void *) compare); \
- }
+ } \
+ \
+ /* End with duplicate declaration, so callers must supply ';' */ \
+ typedef struct name name
#define empty_vector { .ptr = NULL, .len = 0, .cap = 0 }
diff --git c/lib/uri.c i/lib/uri.c
index 367621d4..31ee90f3 100644
--- c/lib/uri.c
+++ i/lib/uri.c
@@ -58,7 +58,7 @@ struct uri_query {
char *value;
};
-DEFINE_VECTOR_TYPE (uri_query_list, struct uri_query)
+DEFINE_VECTOR_TYPE (uri_query_list, struct uri_query);
/* Parse the query_raw substring of a URI into a list of decoded queries.
* Return 0 on success or -1 on error.
diff --git c/copy/file-ops.c i/copy/file-ops.c
index 18cae74a..1efece26 100644
--- c/copy/file-ops.c
+++ i/copy/file-ops.c
@@ -64,7 +64,7 @@
#endif
#ifdef PAGE_CACHE_MAPPING
-DEFINE_VECTOR_TYPE (byte_vector, uint8_t)
+DEFINE_VECTOR_TYPE (byte_vector, uint8_t);
#endif
static struct rw_ops file_ops;
diff --git c/copy/nbd-ops.c i/copy/nbd-ops.c
index 34ab4857..0980a5ed 100644
--- c/copy/nbd-ops.c
+++ i/copy/nbd-ops.c
@@ -33,7 +33,7 @@
static struct rw_ops nbd_ops;
-DEFINE_VECTOR_TYPE (handles, struct nbd_handle *)
+DEFINE_VECTOR_TYPE (handles, struct nbd_handle *);
struct rw_nbd {
struct rw rw;
diff --git c/dump/dump.c i/dump/dump.c
index d0da2879..922bd435 100644
--- c/dump/dump.c
+++ i/dump/dump.c
@@ -38,7 +38,7 @@
#include "version.h"
#include "vector.h"
-DEFINE_VECTOR_TYPE (uint32_vector, uint32_t)
+DEFINE_VECTOR_TYPE (uint32_vector, uint32_t);
static const char *progname;
static struct nbd_handle *nbd;
diff --git c/fuse/nbdfuse.h i/fuse/nbdfuse.h
index 371ca8bb..93b66aac 100644
--- c/fuse/nbdfuse.h
+++ i/fuse/nbdfuse.h
@@ -30,7 +30,7 @@
#include "vector.h"
-DEFINE_VECTOR_TYPE (handles, struct nbd_handle *)
+DEFINE_VECTOR_TYPE (handles, struct nbd_handle *);
extern handles nbd;
extern unsigned connections;
diff --git c/info/list.c i/info/list.c
index c2741ba0..4b4e8f86 100644
--- c/info/list.c
+++ i/info/list.c
@@ -35,7 +35,7 @@ struct export {
char *name;
char *desc;
};
-DEFINE_VECTOR_TYPE (exports, struct export)
+DEFINE_VECTOR_TYPE (exports, struct export);
static exports export_list = empty_vector;
static int
diff --git c/info/map.c i/info/map.c
index a5aad955..4924866a 100644
--- c/info/map.c
+++ i/info/map.c
@@ -36,7 +36,7 @@
#include "nbdinfo.h"
-DEFINE_VECTOR_TYPE (uint32_vector, uint32_t)
+DEFINE_VECTOR_TYPE (uint32_vector, uint32_t);
static void print_extents (uint32_vector *entries);
static void print_totals (uint32_vector *entries, int64_t size);
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org