Re: [PATCH] style(5): No struct typedefs
>> [...] as I see it the divide you're sketching here ([...]) is the >> divide between interface and implementation, and in some cases the >> interface is more than just the typedefs. > Sort of. > // contains the "vnode_t" opaque type definition > // contains the guts of "struct vnode" and the other > implementation details > // Contains some of the file system interfaces, some of which > use vnode_t > // Contains the vnode interfaces, which definitely use vnode_t > The latter if the two would each include . You're right, I hadn't fully understood you. Hmm. What value is provided by separating the vnode_t type from the rest of the vnode interface (in )? If taken to its logical extreme (which of course ends up at a satirical position, like most logical extremes), this leads to // vnode_t // enum vtype // enum vtagtype // #define VNODE_TAGS // struct vnlock // #define IO_UNIT // #define IO_APPEND which I hope you agree is madness. What makes it worth singling out vnode_t for special treatment here? I would prefer to draw include-file boundaries more or less matching conceptual-API boundaries, which I thought was more or less where we started: defines the API to the vnode subsystem, including types, #defines, externs, etc. But I'm not sure how that would differ from what we have now. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: [PATCH] style(5): No struct typedefs
> On Jul 12, 2023, at 5:25 AM, Mouse wrote: > > [paragraph-length-line damage repaired manually] > >> What about something like and >> ? The type definitions go into the former >> header file, [...] > > Well, I don't like the "typedefs" name, Yah, I don't like it either, but.. :-) > because as I see it the divide > you're sketching here (which I support, in general) is the divide > between interface and implementation, and in some cases the interface > is more than just the typedefs. Sort of. // contains the "vnode_t" opaque type definition // contains the guts of "struct vnode" and the other implementation details // Contains some of the file system interfaces, some of which use vnode_t // Contains the vnode interfaces, which definitely use vnode_t The latter if the two would each include . Feel free to pick a better name, but that's the idea. -- thorpej
Re: [PATCH] style(5): No struct typedefs
> I'm tempted to say that an opaque struct declaration in a .c file
> ought be treated suspiciously -
Depending on what you mean by "an opaque struct declaration", I may
agree or disagree.
If you mean the "struct foo;" without a body, then I think I agree.
But the "struct foo { ... };" that completes the incomplete type
corresponding to the include file's "struct foo;", that I think is the
whole point of opaque structs: the completion is available only under
the implementation hood. While that may be in a foo-impl.h file, if
only a single file needs it I see no harm in the completion being in
the .c (and indeed I've done it that way often enough).
> (and it would be kind of nice if C had a way to say "all functions
> defined beyond this point are static").
Personally, I'd prefer "all functions defined before this point are
static", since I prefer calls to move textually backward (or, to put it
another way, I prefer to avoid forward declarations when possible).
But I doubt either of those will appear anytime soon.
> And to return briefly, to the issue way up the top of simplifying the
> leader files, there is one change I've wanted to make for ages, but
> just haven't been brave enough to do,
> That is to rip the definition of __NetBSD_Version__ [...] into a new
> header file all of its own [...] with the rule that *no* other header
> file is oermitted to include it.
I'm...not sure I agree with that.
I once built a kernel facility which I wanted to be completely drop-in
to multiple kernel versions (as widely disparate as 1.4T and 5.2). The
design I came up with was (names probably changed; I'm not digging up
the code to see what names I actually used) a "version.h" file which
looked like
#include // to get __NetBSD_Version__
#if __NetBSD_Version__ == whatever value 1.4T had
#include // where 1.4T keeps it
#define THINGY() do { ...1.4T code for THINGY() ... } while (0)
typedef int COMMON_TYPE; // just an int on 1.4T
#endif
#if __NetBSD_Version__ == whatever value 5.2 had
#include // on 5.2 we need two
#include // include files
#define THINGY() thingy() // 5.2 has THINGY() nicely encapsulated
typedef struct something COMMON_TYPE; // 5.2 uses a struct
#endif
etc. It sounds as though you would prefer the #include that pulls in
__NetBSD_Version__ be in every C file that wants to include
"version.h". This seems counterintuitive, even counterproductive, to
me (and see also below, about #include order). Or perhaps you'd prefer
that I'd designed those interfaces some other way, rather than with a
version-switch include file?
My own pet include-file peeve is rather different: I strongly believe
that, with very few exceptions ( is the only one that comes
to mind), you should be able to re-order your #include lines
arbitrarily without producing any semantic change, and that, if this is
not so, at least one of the include files involved is broken. I've
been making small steps towards fixing this in my own trees, but it's
still a major mess.
/~\ The ASCII Mouse
\ / Ribbon Campaign
X Against HTMLmo...@rodents-montreal.org
/ \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: [PATCH] style(5): No struct typedefs
[paragraph-length-line damage repaired manually] > What about something like and > ? The type definitions go into the former > header file, [...] Well, I don't like the "typedefs" name, because as I see it the divide you're sketching here (which I support, in general) is the divide between interface and implementation, and in some cases the interface is more than just the typedefs. Some structs have their struct definition, or some of it (regex_t is an example), as part of their advertised interface, and many have #defines as well. But I'd rather see the division done with (what I see as) an inaccurate name than see the division not done. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: [PATCH] style(5): No struct typedefs
> On Jul 11, 2023, at 9:51 PM, Robert Elz wrote: > > That is to rip the definition of __NetBSD_Version__ (with however > nany underscores it really has) out of and into a new > header file all of its own (perhaps - no bikeshedding > about names here please) with the rule that *no* other header file is > oermitted to include it. C code that needs __NetBSD_Version__ needs > to explcitly include that file itself ... there is surprisingly little > of it. This will make the side effects of doing a kernel version bump > be so small (really, so it should be) that doing one is an automatic > decision any time it might be needed, and we end the "even though this > is a kernel internal ABI change, I don't think any modules will be affected" > (and no no bump happens - which must be what happens sometimes, either that > of some of our developers don't understand what a kernal internal ABI > change means) and should also avoid the need for "ride the kernel > version bump" (perhaps several hours later) or "let me know when you're > going to bump the kernel version, I have changes to commit which would > need that, but don't need to be commmitted right away" - which often > turns into a "ride the bump" when the developer who did a change, and > a bump, did not remeber, or perhaps even know, the other developer was > waiting and would have likes to coordinate. I’ve been thinking about this a little more… What about something like and ? The type definitions go into the former header file, that can be included by whatever other header needs that type definition. The full-blown structure and internal details of it go into the latter, to be only included by things that access the guts, and now it’s VERY clear that “I am using implementation details” aspect of touching those guts. -- thorpej
