> Date: Tue, 11 Jul 2023 19:50:19 -0700 > From: Jason Thorpe <thor...@me.com> > > > On Jul 11, 2023, at 2:56 PM, Taylor R Campbell > > <campbell+netbsd-tech-k...@mumble.net> wrote: > > > > I agree the keyword is ugly, and it's unfortunate that in order to > > omit it we would have to use C++, but the ugliness gives us practical > > benefits of better type-checking, reduced header file maintenance > > burden, and reduced motivation for unnecessary header file > > dependencies. > > No -- you just don't have to use "void *". Can you point to a > practical problematic example?
Using `struct bus_dma_tag *' instead of `void *' (whether via the bus_dma_tag_t alias or not) would provide better type-checking. Exposing `struct bus_dma_tag *' instead of bus_dma_tag_t as part of the API would reduce header file maintenance burden and unnecessary header file dependencies. But you folks are right that bus_dma_tag_t was confusing, because there are two different issues behind it. kmutex_t is a better example of the header file dependency issue, because a lot of the kernel just passes around pointers to struct kmutex and doesn't need the definition, like sys/condvar.h. This can be served by `struct kmutex *', which doesn't require #include <sys/mutex.h>, instead of `kmutex_t *', which does. The `struct kmutex' definition is particularly difficult, but since callers statically allocate them, must be exposed. In other cases, like pserialize_t, the definition can be completely opaque. A header file that just needs to store a pserialize_t or percpu_t reference would have no need to pull in sys/pserialize.h or sys/percpu.h if we just used `struct pserialize *' or `struct percpu *' instead. (Quiz: Without looking, between pserialize_t and percpu_t, which one is a pointer type and which one is a struct type you have to use with *, or is it both or neither?) > Date: Wed, 12 Jul 2023 11:51:41 +0700 (ICT) > From: Robert Elz <k...@munnari.oz.au> > > But I 100% disagree with the notion of declaring those opaque > struct types over and over again in each individual source file. > Declarations should appear exactly once - either in a source flle > if the object concerned is used only in that file. Otherwise in > a header file. Aside from a quirk of C scope rules for tags[*], there is no semantic content to the `struct s;' forward declaration, so it's harmless to repeat it -- if `struct s' figures into a public part of the API, the `struct s' aspect of that can't get out of sync no matter how many times you write a `struct s' forward declaration. Were it not for that quirk, it wouldn't even be necessary to have the forward declaration in the first place! In contrast, one compilation unit could have `typedef struct s s_t;' while another has `typedef struct s *s_t;' and a third has `typedef void *s_t;'. If the public part of the API is `s_t' instead of `struct s', these could get out of sync. That's why it's necessary for `s_t' to have a single definition in a header file. But if the public part of the API is to use `struct s' instead of `s_t', that can't be a problem. So it is safe for function declarations that pass around pointers to `struct s', or for struct definitions with pointers to `struct s' as members, to skip the header file. [*] In principle, there wouldn't even really be a need for these forward declarations, but for a peculiar clause of C scoping for tags (C11, Ch. 6 `Language', Sec. 6.2 `Concepts', Subsec. 6.2.1 `Scopes of identifiers', paragraph 4, pp. 35-35): If the declarator or type specifier that declares the identifier appears within the list of parameter declarations in a function prototype (not part of a function definition), the identifier has function prototype scope, which terminates at the end of the function declarator. As a consequence, you can't do something like this, because the two `struct s' tags are distinct types: int foo(struct s *); int (*bar)(struct s *) = &foo; But you can do this: struct s; int foo(struct s *); int (*bar)(struct s *) = &foo; > I'm not even sure it is practically possible (in many cases anyway) > to do it as you're suggesting, as to use an opaque struct that way, > you're almost certainly using it as the parameter of one of more > public functions in your source file, in which case those need to > be declared in a header file, and so that neader file also needs > the opaque struct definition. Here's an example where I fixed the build by doing nothing but replacing #include <sys/mutex.h> --> struct kmutex; kmutex_t *...; --> struct kmutex *...; (and fixing some downstream accidents of transitive inclusion): https://mail-index.netbsd.org/source-changes/2023/07/13/msg145972.html This is far from the first time I've solved problems just by making this substitution. The style rule would obviate the need for most build fixes like this because it would be the default way to do things. > I'm tempted to say that an opaque struct declaration in a .c > file ought be treated suspiciously I tend to agree. What part of the proposal makes you think this is likely to happen in .c files? I expect it to affect .h files by obviating the need to include other .h files. [everything else premised on putting these forward declarations in .c files elided] > Eg: (and deliberately using an absurd example, to avoid people > trying to correct my misunderstandings of any real examples) Maybe a real-world example would be more compelling -- but more to the point, you're presenting a contrived counterexample as if I proposed a theorem about all cases. A general style rule can have exceptions; what's important is the common cases, not the contrived counterexamples. > Date: Wed, 12 Jul 2023 13:40:27 -0400 (EDT) > From: Mouse <mo...@rodents-montreal.org> > > Hmm. What value is provided by separating the vnode_t type from the > rest of the vnode interface (in <sys/vnode.h>)? It reduces header file dependencies, reducing incremental build costs, reducing maintenance burden of header files, and reducing accidental transitive inclusions. The last week has been spent disentangling a nightmare of these dependencies to get the build going again, after I added some very useful functionality to crash(8) which meant exposing machine/mutex.h to userland. You can see a snapshot of the difficulty here: https://web.archive.org/web/20230713220210/https://releng.netbsd.org/cgi-bin/builds.cgi But there's a simpler approach which doesn't require making decisions about these boundaries. The approach is to use `struct vnode *' instead of `vnode_t *', which completely dispenses with the question of what header file it has to come from because it doesn't have to come from a header file at all. Sometimes it is worthwhile to make decisions about how to split up header files. But the proposed rule obviates the need to make those decisions in many common cases of passing around pointers to objects that are opaque to most or all users.