I propose the attached change to KNF style(5) to advise against
typedefs for structs or unions, or pointers to them.
Passing around pointers to structs and unions doesn't require the
definition, only a forward declaration:
struct vnode;
int frobnitz(struct vnode *);
This can dramatically cut down on header file dependencies so that
touching one header file doesn't cause the entire kernel to rebuild.
This also makes it clearer whether you are passing around a copy of a
struct or a pointer to a struct, which is often semantically important
not to conceal.
Typedefs are not necessary for opaque cookies to have labels, like
`typedef void *bus_dma_tag_t;'. In fact, using void * here is bad
because it prevents the C compiler from checking types; bus_dma_tag_t
is indistinguishable from audio_dai_tag_t, and from pckbc_tag_t, and
from acpipmtimer_t, and from sdmmc_chipset_handle_t.
If we used `struct bus_dma_tag *' instead, the forward declaration
could be `struct bus_dma_tag;' instead of having to pull in all of
sys/bus.h, _and_ the C compiler would actually check types. There
isn't even any need to define `struct bus_dma_tag' anywhere; the
pointer can be cast in sys/arch/x86/x86/bus_dma.c to `struct
x86_bus_dma_tag', for instance (at some risk in type safety, but a
much smaller risk than having void * as the public interface), and it
doesn't even violate strict aliasing rules.
(Typedefs for integer, function, and function pointer types are not
covered by this advice.)
Objections?
>From 69dc9658454c8edce695630d2e1bc75db86c43eb Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastr...@netbsd.org>
Date: Tue, 11 Jul 2023 09:57:54 +0000
Subject: [PATCH] style(5): Prohibit new struct typedefs and explain why.
---
share/misc/style | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/share/misc/style b/share/misc/style
index 1c02dccecf2a..85790024f3f0 100644
--- a/share/misc/style
+++ b/share/misc/style
@@ -130,6 +130,13 @@ int frobnicate(const char *);
/* Then, there's a blank line, and the user include files. */
#include "pathnames.h" /* Local includes in double quotes. */
+/*
+ * Forward declarations for struct (and union) tags that don't need
+ * definitions go next.
+ */
+struct dirent;
+struct statfs;
+
/*
* Declarations for file-static functions go at the top of the file.
* Don't associate a name with the parameter types. I.e. use:
@@ -207,10 +214,17 @@ struct foo {
};
struct foo *foohead; /* Head of global foo list */
-/* Make the structure name match the typedef. */
+/*
+ * Do not create typedefs for structs or pointers to structs. Using a
+ * typedef obscures whether the object is a struct or a pointer to a
+ * struct, and creates unnecessary header file dependencies when only
+ * pointers to them are passed around.
+ */
+#ifdef BAD
typedef struct BAR {
int level;
} BAR;
+#endif
/* C99 uintN_t is preferred over u_intN_t. */
uint32_t zero;
