9 ms·

OpenBSD kernel source file style guide

112 points by zytek 12y ago

kps 12y ago
The first published BSD KNF can be found various places including here: https://stuff.mit.edu/afs/athena/astaff/reference/4.4lite/usr/src/admin/style/style 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.
deleted 12y ago
[deleted]
feld 12y ago
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 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 :-)
- Alupis 12y ago
  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.
  - xfs 12y ago
    Linux kernel does have a lot of non-functional formatting commits. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/?qt=grep&q=formatting 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.
    - Alupis 12y ago
      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.
      - 3JPLW 12y ago
        Hunh? Try the staging: unisys: fix formatting in timskmod.h patch - e5700df52 [1] 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; (This commit just happened to be the only one I peeked at from the link above - and is particularly apropos) 1. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e5700df5238577e4e570d08a8ee1aa126731dae7 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux....
        Alupis 12y ago
        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).
        cesarb 12y ago
        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.
  - feld 12y ago
    If style issues are missed in FreeBSD before commit they're graciously accepted afterwards from what I've witnessed
  - feld 12y ago
    Taking in large contributions that violate style(9) is a really bad idea, though.
bigfoot 12y ago
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.
- 0x0 12y ago
  What would be a reason for leaving out the parameter names? Genuinely curious.
  - foxhill 12y ago
    DRY principle - you give the variables a name once, in the function definition. e.g: //decl in the header int pow(int exponent, int base); if you only look at the header you might think that the arguments are one way, but //actual definition int pow(int base, int exponent){ //math is correct but base <-> exponent.. } i'm not saying i agree with this at all - it's a contrived example..!
    - tedunangst 12y ago
      CVE-2014-3956
      - erhardm 12y ago
        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.
        bigfoot 12y ago
        OK, agreed. C (and C++) just suck regarding this detail.
ja30278 12y ago
I never understand the desire to omit braces in single line blocks. sure for () foo saves you a line, but it's a bug waiting to happen.
- oneeyedpigeon 12y ago
  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.
- DrJokepu 12y ago
  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.
  - Arnt 12y ago
    Apple's goto/fail bug could only exist because single-line then clauses work.
  - hobo_mark 12y ago
    Oh yes it does. https://www.imperialviolet.org/2014/02/22/applebug.html https://www.imperialviolet.org/2014/02/22/applebug.html
    - DrJokepu 12y ago
      Fair enough. That being said, -Wunreachable-code would have caught that.
      - coolsunglasses 12y ago
        >Fair enough. That being said, -Wunreachable-code would have caught that. -Wunreachable-code was disabled in gcc years ago.
bjackman 12y ago
I'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/LIST_EMPTY.3?query=queue&sec=3&arch=i386 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);"
- jpfr 12y ago
  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.
bjackman 12y ago
I'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/LIST_EMPTY.3?query=queue&sec=3&arch=i386 http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/.... Nice. On the other hand, my experience with BSD code is that it is completely unreadable. Linux has it a bit too, but in BSD there are way too many statements like "mst_fqd->f_do_skb((struct mfq_t *) q);"
mheiler 12y ago
I like that it's short.
JelteF 12y ago
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.
- Negitivefrags 12y ago
  Tabs for indentation, spaces for alignment. This is the only sane standard.
  - huhtenberg 12y ago
    I still find it discomforting that it mandates the use of tabs in the middle of the line - void<tab>function(int); 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.
gcb0 12y ago
openbsd is the only project which manpages are not mostly useless. You have even a starting guide there.
- ninjin 12y ago
  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_manpages https://github.com/ninjin/ppod/tree/master/hck/openbsd_manpa...
- dredmorbius 12y ago
  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).
fredmorcos 12y ago
Here's a question, since the man page mentions splint. Has anyone here ever managed to reach a splint warning-free project?

112 points by zytek 12y ago

kps 12y ago

The first published BSD KNF can be found various places including here: https://stuff.mit.edu/afs/athena/astaff/reference/4.4lite/usr/src/admin/style/style 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.

deleted 12y ago

[deleted]

feld 12y ago

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 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 :-)

Alupis 12y ago
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.
- xfs 12y ago
  Linux kernel does have a lot of non-functional formatting commits. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/?qt=grep&q=formatting 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.
  - Alupis 12y ago
    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.
    - 3JPLW 12y ago
      Hunh? Try the staging: unisys: fix formatting in timskmod.h patch - e5700df52 [1] 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; (This commit just happened to be the only one I peeked at from the link above - and is particularly apropos) 1. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e5700df5238577e4e570d08a8ee1aa126731dae7 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux....
      - Alupis 12y ago
        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).
        cesarb 12y ago
        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.
- feld 12y ago
  If style issues are missed in FreeBSD before commit they're graciously accepted afterwards from what I've witnessed
- feld 12y ago
  Taking in large contributions that violate style(9) is a really bad idea, though.

bigfoot 12y ago

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.

0x0 12y ago
What would be a reason for leaving out the parameter names? Genuinely curious.
- foxhill 12y ago
  DRY principle - you give the variables a name once, in the function definition. e.g: //decl in the header int pow(int exponent, int base); if you only look at the header you might think that the arguments are one way, but //actual definition int pow(int base, int exponent){ //math is correct but base <-> exponent.. } i'm not saying i agree with this at all - it's a contrived example..!
  - tedunangst 12y ago
    CVE-2014-3956
    - erhardm 12y ago
      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.
      - bigfoot 12y ago
        OK, agreed. C (and C++) just suck regarding this detail.

ja30278 12y ago

I never understand the desire to omit braces in single line blocks. sure for () foo saves you a line, but it's a bug waiting to happen.

oneeyedpigeon 12y ago
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.
DrJokepu 12y ago
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.
- Arnt 12y ago
  Apple's goto/fail bug could only exist because single-line then clauses work.
- hobo_mark 12y ago
  Oh yes it does. https://www.imperialviolet.org/2014/02/22/applebug.html https://www.imperialviolet.org/2014/02/22/applebug.html
  - DrJokepu 12y ago
    Fair enough. That being said, -Wunreachable-code would have caught that.
    - coolsunglasses 12y ago
      >Fair enough. That being said, -Wunreachable-code would have caught that. -Wunreachable-code was disabled in gcc years ago.

bjackman 12y ago

I'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/LIST_EMPTY.3?query=queue&sec=3&arch=i386 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);"

jpfr 12y ago
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.

bjackman 12y ago

I'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/LIST_EMPTY.3?query=queue&sec=3&arch=i386 http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/.... Nice. On the other hand, my experience with BSD code is that it is completely unreadable. Linux has it a bit too, but in BSD there are way too many statements like "mst_fqd->f_do_skb((struct mfq_t *) q);"

mheiler 12y ago

I like that it's short.

JelteF 12y ago

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.

Negitivefrags 12y ago
Tabs for indentation, spaces for alignment. This is the only sane standard.
- huhtenberg 12y ago
  I still find it discomforting that it mandates the use of tabs in the middle of the line - void<tab>function(int); 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.

gcb0 12y ago

openbsd is the only project which manpages are not mostly useless. You have even a starting guide there.

ninjin 12y ago
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_manpages https://github.com/ninjin/ppod/tree/master/hck/openbsd_manpa...
dredmorbius 12y ago
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).

fredmorcos 12y ago

Here's a question, since the man page mentions splint. Has anyone here ever managed to reach a splint warning-free project?