Re: [PATCH] style(5): No struct typedefs

[Andrew Cagney <andrew.cagney%gmail.com@localhost>]

On Tue, 11 Jul 2023 at 06:17, Taylor R Campbell <riastradh%netbsd.org@localhost> wrote:
>
> I propose the attached change to KNF style(5) to advise against
> typedefs for structs or unions, or pointers to them.

Perhaps be positive?  Add a section encouraging the use of struct,
union and enum forward declarations when the details aren't needed by
the header?

- I mention enums as compilers are finally getting good at checking them.
- I've found that, over time, the number of unexplainable #includes
does go down; but only when there's a willingness to go in and rip out
the #include spaghetti in .c files

> Passing around pointers to structs and unions doesn't require the
> definition, only a forward declaration:
>
> struct vnode;
> int frobnitz(struct vnode *);

Be clear about where the forward declaration should go.  Once, near
the top after the required #includes, or inline.
Also keep in mind that, just like #includes, these seem to accumulate
over time; especially when inline.

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

If a generic header were to forward declare `struct bus_dma_tag;` as
part of an abstract interface, is each implementation allowed to
provide their own local definition?  It is valid C.  The result can be
a good thing, or a dangerous thing.  Be clear either way.

(constructs such as <<struct foo { int i; }>> were often frowned upon
because the code was crap; that's no longer true; doesn't make them a
good idea though)

I can see your point about typedef, void*, and hiding pointers, but
there are always exceptions.  For instance say the object in question
is an opaque singleton.  This week it is <<typedef enum singleton
single_t>> but next week it is <<typedef const struct singleton
*single_t>>.

> (Typedefs for integer, function, and function pointer types are not
> covered by this advice.)
>
> Objections?

Just get a good lock.