OpenBSD kernel source file style guide
openbsd.orgI've never written code with it, but the BSDs use these macros to implement rudimentary generic types in C: http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/.... Nice.
However, I've only had horrible experiences trying to read the BSDs' kernel code. There are way too many statements like "mst_fqd->f_do_skb((struct mfq_t *) q);"
Yes! That's a really good implementations of linked lists using only preprocessor macros.
sys/queue.h is also provided in all Linux systems.
Easy to use, type-checked by the compiler and a lot faster than a "generic" linked list with pointers to the data. Here, the next/previous pointers get embedded in the payload struct itself.
Linux's is very long in comparison to the BSD's. It seems to have weird edge-cases and possibly unnecessary explanations.
https://www.kernel.org/doc/Documentation/CodingStyle
Example: Why is the comment style different in net/? It seems to serve no obvious purpose.
Too many cooks :-)
Likely because /net was done before the coding standard and by some 3rd party contributor -- and doing non-functional whitespace/formatting commits is a no-no in most large projects.
Linux kernel does have a lot of non-functional formatting commits. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux....
These non-functional commits happen more in areas under active development, while established and inactive projects with less than enough watching eyes want to avoid this kind of non-essential changes.
the link you provided has no "just formatting only" changes. The non-functional changes you see there are updating documentation in comments. That is a big difference. You won't ever see "i just felt like we should have an extra blank line here" changes.
Hunh? Try the staging: unisys: fix formatting in timskmod.h patch - e5700df52 [1]
(This commit just happened to be the only one I peeked at from the link above - and is particularly apropos)diff --git a/drivers/staging/unisys/include/timskmod.h b/drivers/staging/unisys/include/timskmod.h index b20fc9d..59144ba 100644 --- a/drivers/staging/unisys/include/timskmod.h +++ b/drivers/staging/unisys/include/timskmod.h @@ -293,6 +293,7 @@ static inline struct cdev *cdev_alloc_init(struct module *owner, const struct file_operations *fops) { struct cdev *cdev = NULL; + cdev = cdev_alloc(); if (!cdev) return NULL;1. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux....
well i stand corrected it seems. although if that is from staging, then it will likely get squashed before merged into mainline (meaning this commit won't actually stay in the history).
Staging is an exception. Staging is a place to put low-quality drivers developed outside the Linux kernel community, to be fixed to Linux kernel standards before being moved into the normal tree. One of the many things done to a driver in staging is to fix the source code formatting. The commits won't get squashed, since it's part of the mainline git repository, which is never squashed.
So yeah, on staging you can expect to see lots of non-functional changes, mixed with functional changes.
If style issues are missed in FreeBSD before commit they're graciously accepted afterwards from what I've witnessed
Taking in large contributions that violate style(9) is a really bad idea, though.
The first published BSD KNF can be found various places including here: https://stuff.mit.edu/afs/athena/astaff/reference/4.4lite/us...
Like the Linux kernel style, this is essentially K&R style, and the style most of Unix was written in.
openbsd is the only project which manpages are not mostly useless. You have even a starting guide there.
FreeBSD's manpages are also quite excellent. I haven't had to refer to NetBSD's but I'd suspect it's also good.
On the Linux side, Debian tends to support manpages better than others (they're required by Policy, though they're not release-critical).
Agreed, this is the very reason I created a small script [1] to install them on non-OpenBSD systems. They are excellent for as references when coding C and to check what is most likely a GNU extension when writing portable shell scripts.
[1]: https://github.com/ninjin/ppod/tree/master/hck/openbsd_manpa...
I never understand the desire to omit braces in single line blocks.
sure
for ()
foo
Are there not examples where a statement is pretty much guaranteed to live on its own within a block? Having said that, my own style is to always include braces, for aesthetic reasons if nothing else.
When I omit braces I also omit the indented line. I would write:
Makes it clear it's ONE statement. At least for me...for () foo;This makes debugging much harder.
A function or class that doesn't fit on a single screen is a bug waiting to happen.
People keep saying that, but do bugs like that really happen? In my entire career as a professional software developer I've never seen it happening. Not once.
Oh yes it does.
Fair enough. That being said, -Wunreachable-code would have caught that.
>Fair enough. That being said, -Wunreachable-code would have caught that.
-Wunreachable-code was disabled in gcc years ago.
Yes, but I would assume that Apple uses clang internally?
Apple's goto/fail bug could only exist because single-line then clauses work.
I like that it's short.
Archaic and impractical. Example: Instead of using Linux' pragmatic approach to function prototypes:
"In function prototypes, include parameter names with their data types. Although this is not required by the C language, it is preferred in Linux because it is a simple way to add valuable information for the reader."
OpenBSD enforces this:
"Prototypes should not have variable names associated with the types; i.e., void function(int); not: void function(int a);"
Instead of letting the code tell the parameters' purposes, this now has to be deduced from informal descriptions, or the function definition in some .c file.
I love how frequently people on HN decry actual real working things people actually work on and use as "impractical". I suspect the combined experience of the entire openbsd team is greater than yours.
What would be a reason for leaving out the parameter names? Genuinely curious.
DRY principle - you give the variables a name once, in the function definition. e.g:
if you only look at the header you might think that the arguments are one way, but//decl in the header int pow(int exponent, int base);
i'm not saying i agree with this at all - it's a contrived example..!//actual definition int pow(int base, int exponent){ //math is correct but base <-> exponent.. }CVE-2014-3956
Replying with a CVE is like icing on the cake. :) It really shows that OpenBSD devs take security seriously. Funny timing, I was just reading Absolute OpenBSD.
It also demonstrates that sometimes, there are additional constraints that you may not understand in choices you may not agree with.
OK, agreed. C (and C++) just suck regarding this detail.
I don't see how that's relevant.
The bug in that CVE is that the function call got the parameter order wrong. The declaration was correct AFAICT, and of course completely irrelevant because you can make that mistake regardless of what the header says.
Parameters in headers are just documentation, by definition. Documentation can be wrong however you write it, but in general it helps to have it instead of not. Would you seriously argue that function parameters should not be given names in documentation?
> The bug in that CVE is that the function call got the parameter order wrong. The declaration was correct AFAICT
Actually if you look at the patch to fix this issue, they swap the identifiers in the declarator. Of course when something like this happens, you're free to choose whether the definition or the callers should be changed.
https://www.FreeBSD.org/security/patches/SA-14:11/sendmail.p...
That's the function definition, not its declaration. It's pre-ANSI C, so that semicolon can mislead. I didn't bother to grab the source to check whatever the header said (or, heh, didn't say, per OpenBSD convention), but I'm going to assume it was correct.
It amuses me greatly that we're sitting here arguing over the minutiae of coding standards when this thing is still written in K&R syntax.
But regardless: whatever the header said, it wouldn't have affected this bug, which was just a straightforward transpose thing between compatible types. People get memcpy() reversed all the time too, and frankly I don't know if I've ever looked at its function declaration in a header.
i would argue that had the function prototype had names for it's arguments that this might not have happened.
Hm. This is the kind of thing where a compiler warning could be useful. I took a quick look at gcc's manual, but it does not seem to have a warning for this particular mistake (argument names do not match between declaration and definition).
That way, you could repeat yourself safely, since the repetition would be checked by the compiler.
I did a quick -Wall test with gcc 4.4.5 and it doesn't warn.
To allow the function definition to serve as the single source of truth. That would be my guess. They simply don't want to have to update the parameter name at N places, if a parameter were to be renamed.
This is the first time I've heard of a style guide that demands a mixture of tabs and spaces. I thought this was very much frowned upon.
Tabs for indentation, spaces for alignment. This is the only sane standard.
I still find it discomforting that it mandates the use of tabs in the middle of the line -
A cleaner option is "tabs for indentation, spaces for alignment", but with tabs allowed at the beginning of line only. This way opening a file in an editor with a different tab size will still preserve the alignment of the parts.void<tab>function(int);
Here's a question, since the man page mentions splint. Has anyone here ever managed to reach a splint warning-free project?