[Libguestfs] [libnbd PATCH v4 3/5] remove semicolon after DEFINE_VECTOR_TYPE() macro invocations

Eric Blake eblake at redhat.com
Fri Feb 24 15:04:27 UTC 2023


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 at 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


More information about the Libguestfs mailing list