| 1 | .. _codingstyle:
|
| 2 |
|
| 3 | Linux kernel coding style
|
| 4 | =========================
|
| 5 |
|
| 6 | This is a short document describing the preferred coding style for the
|
| 7 | linux kernel. Coding style is very personal, and I won't **force** my
|
| 8 | views on anybody, but this is what goes for anything that I have to be
|
| 9 | able to maintain, and I'd prefer it for most other things too. Please
|
| 10 | at least consider the points made here.
|
| 11 |
|
| 12 | First off, I'd suggest printing out a copy of the GNU coding standards,
|
| 13 | and NOT read it. Burn them, it's a great symbolic gesture.
|
| 14 |
|
| 15 | Anyway, here goes:
|
| 16 |
|
| 17 |
|
| 18 | 1) Indentation
|
| 19 | --------------
|
| 20 |
|
| 21 | Tabs are 8 characters, and thus indentations are also 8 characters.
|
| 22 | There are heretic movements that try to make indentations 4 (or even 2!)
|
| 23 | characters deep, and that is akin to trying to define the value of PI to
|
| 24 | be 3.
|
| 25 |
|
| 26 | Rationale: The whole idea behind indentation is to clearly define where
|
| 27 | a block of control starts and ends. Especially when you've been looking
|
| 28 | at your screen for 20 straight hours, you'll find it a lot easier to see
|
| 29 | how the indentation works if you have large indentations.
|
| 30 |
|
| 31 | Now, some people will claim that having 8-character indentations makes
|
| 32 | the code move too far to the right, and makes it hard to read on a
|
| 33 | 80-character terminal screen. The answer to that is that if you need
|
| 34 | more than 3 levels of indentation, you're screwed anyway, and should fix
|
| 35 | your program.
|
| 36 |
|
| 37 | In short, 8-char indents make things easier to read, and have the added
|
| 38 | benefit of warning you when you're nesting your functions too deep.
|
| 39 | Heed that warning.
|
| 40 |
|
| 41 | The preferred way to ease multiple indentation levels in a switch statement is
|
| 42 | to align the ``switch`` and its subordinate ``case`` labels in the same column
|
| 43 | instead of ``double-indenting`` the ``case`` labels. E.g.:
|
| 44 |
|
| 45 | .. code-block:: c
|
| 46 |
|
| 47 | switch (suffix) {
|
| 48 | case 'G':
|
| 49 | case 'g':
|
| 50 | mem <<= 30;
|
| 51 | break;
|
| 52 | case 'M':
|
| 53 | case 'm':
|
| 54 | mem <<= 20;
|
| 55 | break;
|
| 56 | case 'K':
|
| 57 | case 'k':
|
| 58 | mem <<= 10;
|
| 59 | fallthrough;
|
| 60 | default:
|
| 61 | break;
|
| 62 | }
|
| 63 |
|
| 64 | Don't put multiple statements on a single line unless you have
|
| 65 | something to hide:
|
| 66 |
|
| 67 | .. code-block:: c
|
| 68 |
|
| 69 | if (condition) do_this;
|
| 70 | do_something_everytime;
|
| 71 |
|
| 72 | Don't use commas to avoid using braces:
|
| 73 |
|
| 74 | .. code-block:: c
|
| 75 |
|
| 76 | if (condition)
|
| 77 | do_this(), do_that();
|
| 78 |
|
| 79 | Always use braces for multiple statements:
|
| 80 |
|
| 81 | .. code-block:: c
|
| 82 |
|
| 83 | if (condition) {
|
| 84 | do_this();
|
| 85 | do_that();
|
| 86 | }
|
| 87 |
|
| 88 | Don't put multiple assignments on a single line either. Kernel coding style
|
| 89 | is super simple. Avoid tricky expressions.
|
| 90 |
|
| 91 |
|
| 92 | Outside of comments, documentation and except in Kconfig, spaces are never
|
| 93 | used for indentation, and the above example is deliberately broken.
|
| 94 |
|
| 95 | Get a decent editor and don't leave whitespace at the end of lines.
|
| 96 |
|
| 97 |
|
| 98 | 2) Breaking long lines and strings
|
| 99 | ----------------------------------
|
| 100 |
|
| 101 | Coding style is all about readability and maintainability using commonly
|
| 102 | available tools.
|
| 103 |
|
| 104 | The preferred limit on the length of a single line is 80 columns.
|
| 105 |
|
| 106 | Statements longer than 80 columns should be broken into sensible chunks,
|
| 107 | unless exceeding 80 columns significantly increases readability and does
|
| 108 | not hide information.
|
| 109 |
|
| 110 | Descendants are always substantially shorter than the parent and
|
| 111 | are placed substantially to the right. A very commonly used style
|
| 112 | is to align descendants to a function open parenthesis.
|
| 113 |
|
| 114 | These same rules are applied to function headers with a long argument list.
|
| 115 |
|
| 116 | However, never break user-visible strings such as printk messages because
|
| 117 | that breaks the ability to grep for them.
|
| 118 |
|
| 119 |
|
| 120 | 3) Placing Braces and Spaces
|
| 121 | ----------------------------
|
| 122 |
|
| 123 | The other issue that always comes up in C styling is the placement of
|
| 124 | braces. Unlike the indent size, there are few technical reasons to
|
| 125 | choose one placement strategy over the other, but the preferred way, as
|
| 126 | shown to us by the prophets Kernighan and Ritchie, is to put the opening
|
| 127 | brace last on the line, and put the closing brace first, thusly:
|
| 128 |
|
| 129 | .. code-block:: c
|
| 130 |
|
| 131 | if (x is true) {
|
| 132 | we do y
|
| 133 | }
|
| 134 |
|
| 135 | This applies to all non-function statement blocks (if, switch, for,
|
| 136 | while, do). E.g.:
|
| 137 |
|
| 138 | .. code-block:: c
|
| 139 |
|
| 140 | switch (action) {
|
| 141 | case KOBJ_ADD:
|
| 142 | return "add";
|
| 143 | case KOBJ_REMOVE:
|
| 144 | return "remove";
|
| 145 | case KOBJ_CHANGE:
|
| 146 | return "change";
|
| 147 | default:
|
| 148 | return NULL;
|
| 149 | }
|
| 150 |
|
| 151 | However, there is one special case, namely functions: they have the
|
| 152 | opening brace at the beginning of the next line, thus:
|
| 153 |
|
| 154 | .. code-block:: c
|
| 155 |
|
| 156 | int function(int x)
|
| 157 | {
|
| 158 | body of function
|
| 159 | }
|
| 160 |
|
| 161 | Heretic people all over the world have claimed that this inconsistency
|
| 162 | is ... well ... inconsistent, but all right-thinking people know that
|
| 163 | (a) K&R are **right** and (b) K&R are right. Besides, functions are
|
| 164 | special anyway (you can't nest them in C).
|
| 165 |
|
| 166 | Note that the closing brace is empty on a line of its own, **except** in
|
| 167 | the cases where it is followed by a continuation of the same statement,
|
| 168 | ie a ``while`` in a do-statement or an ``else`` in an if-statement, like
|
| 169 | this:
|
| 170 |
|
| 171 | .. code-block:: c
|
| 172 |
|
| 173 | do {
|
| 174 | body of do-loop
|
| 175 | } while (condition);
|
| 176 |
|
| 177 | and
|
| 178 |
|
| 179 | .. code-block:: c
|
| 180 |
|
| 181 | if (x == y) {
|
| 182 | ..
|
| 183 | } else if (x > y) {
|
| 184 | ...
|
| 185 | } else {
|
| 186 | ....
|
| 187 | }
|
| 188 |
|
| 189 | Rationale: K&R.
|
| 190 |
|
| 191 | Also, note that this brace-placement also minimizes the number of empty
|
| 192 | (or almost empty) lines, without any loss of readability. Thus, as the
|
| 193 | supply of new-lines on your screen is not a renewable resource (think
|
| 194 | 25-line terminal screens here), you have more empty lines to put
|
| 195 | comments on.
|
| 196 |
|
| 197 | Do not unnecessarily use braces where a single statement will do.
|
| 198 |
|
| 199 | .. code-block:: c
|
| 200 |
|
| 201 | if (condition)
|
| 202 | action();
|
| 203 |
|
| 204 | and
|
| 205 |
|
| 206 | .. code-block:: c
|
| 207 |
|
| 208 | if (condition)
|
| 209 | do_this();
|
| 210 | else
|
| 211 | do_that();
|
| 212 |
|
| 213 | This does not apply if only one branch of a conditional statement is a single
|
| 214 | statement; in the latter case use braces in both branches:
|
| 215 |
|
| 216 | .. code-block:: c
|
| 217 |
|
| 218 | if (condition) {
|
| 219 | do_this();
|
| 220 | do_that();
|
| 221 | } else {
|
| 222 | otherwise();
|
| 223 | }
|
| 224 |
|
| 225 | Also, use braces when a loop contains more than a single simple statement:
|
| 226 |
|
| 227 | .. code-block:: c
|
| 228 |
|
| 229 | while (condition) {
|
| 230 | if (test)
|
| 231 | do_something();
|
| 232 | }
|
| 233 |
|
| 234 | 3.1) Spaces
|
| 235 | ***********
|
| 236 |
|
| 237 | Linux kernel style for use of spaces depends (mostly) on
|
| 238 | function-versus-keyword usage. Use a space after (most) keywords. The
|
| 239 | notable exceptions are sizeof, typeof, alignof, and __attribute__, which look
|
| 240 | somewhat like functions (and are usually used with parentheses in Linux,
|
| 241 | although they are not required in the language, as in: ``sizeof info`` after
|
| 242 | ``struct fileinfo info;`` is declared).
|
| 243 |
|
| 244 | So use a space after these keywords::
|
| 245 |
|
| 246 | if, switch, case, for, do, while
|
| 247 |
|
| 248 | but not with sizeof, typeof, alignof, or __attribute__. E.g.,
|
| 249 |
|
| 250 | .. code-block:: c
|
| 251 |
|
| 252 |
|
| 253 | s = sizeof(struct file);
|
| 254 |
|
| 255 | Do not add spaces around (inside) parenthesized expressions. This example is
|
| 256 | **bad**:
|
| 257 |
|
| 258 | .. code-block:: c
|
| 259 |
|
| 260 |
|
| 261 | s = sizeof( struct file );
|
| 262 |
|
| 263 | When declaring pointer data or a function that returns a pointer type, the
|
| 264 | preferred use of ``*`` is adjacent to the data name or function name and not
|
| 265 | adjacent to the type name. Examples:
|
| 266 |
|
| 267 | .. code-block:: c
|
| 268 |
|
| 269 |
|
| 270 | char *linux_banner;
|
| 271 | unsigned long long memparse(char *ptr, char **retptr);
|
| 272 | char *match_strdup(substring_t *s);
|
| 273 |
|
| 274 | Use one space around (on each side of) most binary and ternary operators,
|
| 275 | such as any of these::
|
| 276 |
|
| 277 | = + - < > * / % | & ^ <= >= == != ? :
|
| 278 |
|
| 279 | but no space after unary operators::
|
| 280 |
|
| 281 | & * + - ~ ! sizeof typeof alignof __attribute__ defined
|
| 282 |
|
| 283 | no space before the postfix increment & decrement unary operators::
|
| 284 |
|
| 285 | ++ --
|
| 286 |
|
| 287 | no space after the prefix increment & decrement unary operators::
|
| 288 |
|
| 289 | ++ --
|
| 290 |
|
| 291 | and no space around the ``.`` and ``->`` structure member operators.
|
| 292 |
|
| 293 | Do not leave trailing whitespace at the ends of lines. Some editors with
|
| 294 | ``smart`` indentation will insert whitespace at the beginning of new lines as
|
| 295 | appropriate, so you can start typing the next line of code right away.
|
| 296 | However, some such editors do not remove the whitespace if you end up not
|
| 297 | putting a line of code there, such as if you leave a blank line. As a result,
|
| 298 | you end up with lines containing trailing whitespace.
|
| 299 |
|
| 300 | Git will warn you about patches that introduce trailing whitespace, and can
|
| 301 | optionally strip the trailing whitespace for you; however, if applying a series
|
| 302 | of patches, this may make later patches in the series fail by changing their
|
| 303 | context lines.
|
| 304 |
|
| 305 |
|
| 306 | 4) Naming
|
| 307 | ---------
|
| 308 |
|
| 309 | C is a Spartan language, and your naming conventions should follow suit.
|
| 310 | Unlike Modula-2 and Pascal programmers, C programmers do not use cute
|
| 311 | names like ThisVariableIsATemporaryCounter. A C programmer would call that
|
| 312 | variable ``tmp``, which is much easier to write, and not the least more
|
| 313 | difficult to understand.
|
| 314 |
|
| 315 | HOWEVER, while mixed-case names are frowned upon, descriptive names for
|
| 316 | global variables are a must. To call a global function ``foo`` is a
|
| 317 | shooting offense.
|
| 318 |
|
| 319 | GLOBAL variables (to be used only if you **really** need them) need to
|
| 320 | have descriptive names, as do global functions. If you have a function
|
| 321 | that counts the number of active users, you should call that
|
| 322 | ``count_active_users()`` or similar, you should **not** call it ``cntusr()``.
|
| 323 |
|
| 324 | Encoding the type of a function into the name (so-called Hungarian
|
| 325 | notation) is asinine - the compiler knows the types anyway and can check
|
| 326 | those, and it only confuses the programmer.
|
| 327 |
|
| 328 | LOCAL variable names should be short, and to the point. If you have
|
| 329 | some random integer loop counter, it should probably be called ``i``.
|
| 330 | Calling it ``loop_counter`` is non-productive, if there is no chance of it
|
| 331 | being mis-understood. Similarly, ``tmp`` can be just about any type of
|
| 332 | variable that is used to hold a temporary value.
|
| 333 |
|
| 334 | If you are afraid to mix up your local variable names, you have another
|
| 335 | problem, which is called the function-growth-hormone-imbalance syndrome.
|
| 336 | See chapter 6 (Functions).
|
| 337 |
|
| 338 | For symbol names and documentation, avoid introducing new usage of
|
| 339 | 'master / slave' (or 'slave' independent of 'master') and 'blacklist /
|
| 340 | whitelist'.
|
| 341 |
|
| 342 | Recommended replacements for 'master / slave' are:
|
| 343 | '{primary,main} / {secondary,replica,subordinate}'
|
| 344 | '{initiator,requester} / {target,responder}'
|
| 345 | '{controller,host} / {device,worker,proxy}'
|
| 346 | 'leader / follower'
|
| 347 | 'director / performer'
|
| 348 |
|
| 349 | Recommended replacements for 'blacklist/whitelist' are:
|
| 350 | 'denylist / allowlist'
|
| 351 | 'blocklist / passlist'
|
| 352 |
|
| 353 | Exceptions for introducing new usage is to maintain a userspace ABI/API,
|
| 354 | or when updating code for an existing (as of 2020) hardware or protocol
|
| 355 | specification that mandates those terms. For new specifications
|
| 356 | translate specification usage of the terminology to the kernel coding
|
| 357 | standard where possible.
|
| 358 |
|
| 359 | 5) Typedefs
|
| 360 | -----------
|
| 361 |
|
| 362 | Please don't use things like ``vps_t``.
|
| 363 | It's a **mistake** to use typedef for structures and pointers. When you see a
|
| 364 |
|
| 365 | .. code-block:: c
|
| 366 |
|
| 367 |
|
| 368 | vps_t a;
|
| 369 |
|
| 370 | in the source, what does it mean?
|
| 371 | In contrast, if it says
|
| 372 |
|
| 373 | .. code-block:: c
|
| 374 |
|
| 375 | struct virtual_container *a;
|
| 376 |
|
| 377 | you can actually tell what ``a`` is.
|
| 378 |
|
| 379 | Lots of people think that typedefs ``help readability``. Not so. They are
|
| 380 | useful only for:
|
| 381 |
|
| 382 | (a) totally opaque objects (where the typedef is actively used to **hide**
|
| 383 | what the object is).
|
| 384 |
|
| 385 | Example: ``pte_t`` etc. opaque objects that you can only access using
|
| 386 | the proper accessor functions.
|
| 387 |
|
| 388 | .. note::
|
| 389 |
|
| 390 | Opaqueness and ``accessor functions`` are not good in themselves.
|
| 391 | The reason we have them for things like pte_t etc. is that there
|
| 392 | really is absolutely **zero** portably accessible information there.
|
| 393 |
|
| 394 | (b) Clear integer types, where the abstraction **helps** avoid confusion
|
| 395 | whether it is ``int`` or ``long``.
|
| 396 |
|
| 397 | u8/u16/u32 are perfectly fine typedefs, although they fit into
|
| 398 | category (d) better than here.
|
| 399 |
|
| 400 | .. note::
|
| 401 |
|
| 402 | Again - there needs to be a **reason** for this. If something is
|
| 403 | ``unsigned long``, then there's no reason to do
|
| 404 |
|
| 405 | typedef unsigned long myflags_t;
|
| 406 |
|
| 407 | but if there is a clear reason for why it under certain circumstances
|
| 408 | might be an ``unsigned int`` and under other configurations might be
|
| 409 | ``unsigned long``, then by all means go ahead and use a typedef.
|
| 410 |
|
| 411 | (c) when you use sparse to literally create a **new** type for
|
| 412 | type-checking.
|
| 413 |
|
| 414 | (d) New types which are identical to standard C99 types, in certain
|
| 415 | exceptional circumstances.
|
| 416 |
|
| 417 | Although it would only take a short amount of time for the eyes and
|
| 418 | brain to become accustomed to the standard types like ``uint32_t``,
|
| 419 | some people object to their use anyway.
|
| 420 |
|
| 421 | Therefore, the Linux-specific ``u8/u16/u32/u64`` types and their
|
| 422 | signed equivalents which are identical to standard types are
|
| 423 | permitted -- although they are not mandatory in new code of your
|
| 424 | own.
|
| 425 |
|
| 426 | When editing existing code which already uses one or the other set
|
| 427 | of types, you should conform to the existing choices in that code.
|
| 428 |
|
| 429 | (e) Types safe for use in userspace.
|
| 430 |
|
| 431 | In certain structures which are visible to userspace, we cannot
|
| 432 | require C99 types and cannot use the ``u32`` form above. Thus, we
|
| 433 | use __u32 and similar types in all structures which are shared
|
| 434 | with userspace.
|
| 435 |
|
| 436 | Maybe there are other cases too, but the rule should basically be to NEVER
|
| 437 | EVER use a typedef unless you can clearly match one of those rules.
|
| 438 |
|
| 439 | In general, a pointer, or a struct that has elements that can reasonably
|
| 440 | be directly accessed should **never** be a typedef.
|
| 441 |
|
| 442 |
|
| 443 | 6) Functions
|
| 444 | ------------
|
| 445 |
|
| 446 | Functions should be short and sweet, and do just one thing. They should
|
| 447 | fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
|
| 448 | as we all know), and do one thing and do that well.
|
| 449 |
|
| 450 | The maximum length of a function is inversely proportional to the
|
| 451 | complexity and indentation level of that function. So, if you have a
|
| 452 | conceptually simple function that is just one long (but simple)
|
| 453 | case-statement, where you have to do lots of small things for a lot of
|
| 454 | different cases, it's OK to have a longer function.
|
| 455 |
|
| 456 | However, if you have a complex function, and you suspect that a
|
| 457 | less-than-gifted first-year high-school student might not even
|
| 458 | understand what the function is all about, you should adhere to the
|
| 459 | maximum limits all the more closely. Use helper functions with
|
| 460 | descriptive names (you can ask the compiler to in-line them if you think
|
| 461 | it's performance-critical, and it will probably do a better job of it
|
| 462 | than you would have done).
|
| 463 |
|
| 464 | Another measure of the function is the number of local variables. They
|
| 465 | shouldn't exceed 5-10, or you're doing something wrong. Re-think the
|
| 466 | function, and split it into smaller pieces. A human brain can
|
| 467 | generally easily keep track of about 7 different things, anything more
|
| 468 | and it gets confused. You know you're brilliant, but maybe you'd like
|
| 469 | to understand what you did 2 weeks from now.
|
| 470 |
|
| 471 | In source files, separate functions with one blank line. If the function is
|
| 472 | exported, the **EXPORT** macro for it should follow immediately after the
|
| 473 | closing function brace line. E.g.:
|
| 474 |
|
| 475 | .. code-block:: c
|
| 476 |
|
| 477 | int system_is_up(void)
|
| 478 | {
|
| 479 | return system_state == SYSTEM_RUNNING;
|
| 480 | }
|
| 481 | EXPORT_SYMBOL(system_is_up);
|
| 482 |
|
| 483 | 6.1) Function prototypes
|
| 484 | ************************
|
| 485 |
|
| 486 | In function prototypes, include parameter names with their data types.
|
| 487 | Although this is not required by the C language, it is preferred in Linux
|
| 488 | because it is a simple way to add valuable information for the reader.
|
| 489 |
|
| 490 | Do not use the ``extern`` keyword with function declarations as this makes
|
| 491 | lines longer and isn't strictly necessary.
|
| 492 |
|
| 493 | When writing function prototypes, please keep the `order of elements regular
|
| 494 | <https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com/>`_.
|
| 495 | For example, using this function declaration example::
|
| 496 |
|
| 497 | __init void * __must_check action(enum magic value, size_t size, u8 count,
|
| 498 | char *fmt, ...) __printf(4, 5) __malloc;
|
| 499 |
|
| 500 | The preferred order of elements for a function prototype is:
|
| 501 |
|
| 502 | - storage class (below, ``static __always_inline``, noting that ``__always_inline``
|
| 503 | is technically an attribute but is treated like ``inline``)
|
| 504 | - storage class attributes (here, ``__init`` -- i.e. section declarations, but also
|
| 505 | things like ``__cold``)
|
| 506 | - return type (here, ``void *``)
|
| 507 | - return type attributes (here, ``__must_check``)
|
| 508 | - function name (here, ``action``)
|
| 509 | - function parameters (here, ``(enum magic value, size_t size, u8 count, char *fmt, ...)``,
|
| 510 | noting that parameter names should always be included)
|
| 511 | - function parameter attributes (here, ``__printf(4, 5)``)
|
| 512 | - function behavior attributes (here, ``__malloc``)
|
| 513 |
|
| 514 | Note that for a function **definition** (i.e. the actual function body),
|
| 515 | the compiler does not allow function parameter attributes after the
|
| 516 | function parameters. In these cases, they should go after the storage
|
| 517 | class attributes (e.g. note the changed position of ``__printf(4, 5)``
|
| 518 | below, compared to the **declaration** example above)::
|
| 519 |
|
| 520 | static __always_inline __init __printf(4, 5) void * __must_check action(enum magic value,
|
| 521 | size_t size, u8 count, char *fmt, ...) __malloc
|
| 522 | {
|
| 523 | ...
|
| 524 | }
|
| 525 |
|
| 526 | 7) Centralized exiting of functions
|
| 527 | -----------------------------------
|
| 528 |
|
| 529 | Albeit deprecated by some people, the equivalent of the goto statement is
|
| 530 | used frequently by compilers in form of the unconditional jump instruction.
|
| 531 |
|
| 532 | The goto statement comes in handy when a function exits from multiple
|
| 533 | locations and some common work such as cleanup has to be done. If there is no
|
| 534 | cleanup needed then just return directly.
|
| 535 |
|
| 536 | Choose label names which say what the goto does or why the goto exists. An
|
| 537 | example of a good name could be ``out_free_buffer:`` if the goto frees ``buffer``.
|
| 538 | Avoid using GW-BASIC names like ``err1:`` and ``err2:``, as you would have to
|
| 539 | renumber them if you ever add or remove exit paths, and they make correctness
|
| 540 | difficult to verify anyway.
|
| 541 |
|
| 542 | The rationale for using gotos is:
|
| 543 |
|
| 544 | - unconditional statements are easier to understand and follow
|
| 545 | - nesting is reduced
|
| 546 | - errors by not updating individual exit points when making
|
| 547 | modifications are prevented
|
| 548 | - saves the compiler work to optimize redundant code away ;)
|
| 549 |
|
| 550 | .. code-block:: c
|
| 551 |
|
| 552 | int fun(int a)
|
| 553 | {
|
| 554 | int result = 0;
|
| 555 | char *buffer;
|
| 556 |
|
| 557 | buffer = kmalloc(SIZE, GFP_KERNEL);
|
| 558 | if (!buffer)
|
| 559 | return -ENOMEM;
|
| 560 |
|
| 561 | if (condition1) {
|
| 562 | while (loop1) {
|
| 563 | ...
|
| 564 | }
|
| 565 | result = 1;
|
| 566 | goto out_free_buffer;
|
| 567 | }
|
| 568 | ...
|
| 569 | out_free_buffer:
|
| 570 | kfree(buffer);
|
| 571 | return result;
|
| 572 | }
|
| 573 |
|
| 574 | A common type of bug to be aware of is ``one err bugs`` which look like this:
|
| 575 |
|
| 576 | .. code-block:: c
|
| 577 |
|
| 578 | err:
|
| 579 | kfree(foo->bar);
|
| 580 | kfree(foo);
|
| 581 | return ret;
|
| 582 |
|
| 583 | The bug in this code is that on some exit paths ``foo`` is NULL. Normally the
|
| 584 | fix for this is to split it up into two error labels ``err_free_bar:`` and
|
| 585 | ``err_free_foo:``:
|
| 586 |
|
| 587 | .. code-block:: c
|
| 588 |
|
| 589 | err_free_bar:
|
| 590 | kfree(foo->bar);
|
| 591 | err_free_foo:
|
| 592 | kfree(foo);
|
| 593 | return ret;
|
| 594 |
|
| 595 | Ideally you should simulate errors to test all exit paths.
|
| 596 |
|
| 597 |
|
| 598 | 8) Commenting
|
| 599 | -------------
|
| 600 |
|
| 601 | Comments are good, but there is also a danger of over-commenting. NEVER
|
| 602 | try to explain HOW your code works in a comment: it's much better to
|
| 603 | write the code so that the **working** is obvious, and it's a waste of
|
| 604 | time to explain badly written code.
|
| 605 |
|
| 606 | Generally, you want your comments to tell WHAT your code does, not HOW.
|
| 607 | Also, try to avoid putting comments inside a function body: if the
|
| 608 | function is so complex that you need to separately comment parts of it,
|
| 609 | you should probably go back to chapter 6 for a while. You can make
|
| 610 | small comments to note or warn about something particularly clever (or
|
| 611 | ugly), but try to avoid excess. Instead, put the comments at the head
|
| 612 | of the function, telling people what it does, and possibly WHY it does
|
| 613 | it.
|
| 614 |
|
| 615 | When commenting the kernel API functions, please use the kernel-doc format.
|
| 616 | See the files at :ref:`Documentation/doc-guide/ <doc_guide>` and
|
| 617 | ``tools/docs/kernel-doc`` for details. Note that the danger of over-commenting
|
| 618 | applies to kernel-doc comments all the same. Do not add boilerplate
|
| 619 | kernel-doc which simply reiterates what's obvious from the signature
|
| 620 | of the function.
|
| 621 |
|
| 622 | The preferred style for long (multi-line) comments is:
|
| 623 |
|
| 624 | .. code-block:: c
|
| 625 |
|
| 626 | /*
|
| 627 | * This is the preferred style for multi-line
|
| 628 | * comments in the Linux kernel source code.
|
| 629 | * Please use it consistently.
|
| 630 | *
|
| 631 | * Description: A column of asterisks on the left side,
|
| 632 | * with beginning and ending almost-blank lines.
|
| 633 | */
|
| 634 |
|
| 635 | It's also important to comment data, whether they are basic types or derived
|
| 636 | types. To this end, use just one data declaration per line (no commas for
|
| 637 | multiple data declarations). This leaves you room for a small comment on each
|
| 638 | item, explaining its use.
|
| 639 |
|
| 640 |
|
| 641 | 9) You've made a mess of it
|
| 642 | ---------------------------
|
| 643 |
|
| 644 | That's OK, we all do. You've probably been told by your long-time Unix
|
| 645 | user helper that ``GNU emacs`` automatically formats the C sources for
|
| 646 | you, and you've noticed that yes, it does do that, but the defaults it
|
| 647 | uses are less than desirable (in fact, they are worse than random
|
| 648 | typing - an infinite number of monkeys typing into GNU emacs would never
|
| 649 | make a good program).
|
| 650 |
|
| 651 | So, you can either get rid of GNU emacs, or change it to use saner
|
| 652 | values. To do the latter, you can stick the following in your .emacs file:
|
| 653 |
|
| 654 | .. code-block:: elisp
|
| 655 |
|
| 656 | (defun c-lineup-arglist-tabs-only (ignored)
|
| 657 | "Line up argument lists by tabs, not spaces"
|
| 658 | (let* ((anchor (c-langelem-pos c-syntactic-element))
|
| 659 | (column (c-langelem-2nd-pos c-syntactic-element))
|
| 660 | (offset (- (1+ column) anchor))
|
| 661 | (steps (floor offset c-basic-offset)))
|
| 662 | (* (max steps 1)
|
| 663 | c-basic-offset)))
|
| 664 |
|
| 665 | (dir-locals-set-class-variables
|
| 666 | 'linux-kernel
|
| 667 | '((c-mode . (
|
| 668 | (c-basic-offset . 8)
|
| 669 | (c-label-minimum-indentation . 0)
|
| 670 | (c-offsets-alist . (
|
| 671 | (arglist-close . c-lineup-arglist-tabs-only)
|
| 672 | (arglist-cont-nonempty .
|
| 673 | (c-lineup-gcc-asm-reg c-lineup-arglist-tabs-only))
|
| 674 | (arglist-intro . +)
|
| 675 | (brace-list-intro . +)
|
| 676 | (c . c-lineup-C-comments)
|
| 677 | (case-label . 0)
|
| 678 | (comment-intro . c-lineup-comment)
|
| 679 | (cpp-define-intro . +)
|
| 680 | (cpp-macro . -1000)
|
| 681 | (cpp-macro-cont . +)
|
| 682 | (defun-block-intro . +)
|
| 683 | (else-clause . 0)
|
| 684 | (func-decl-cont . +)
|
| 685 | (inclass . +)
|
| 686 | (inher-cont . c-lineup-multi-inher)
|
| 687 | (knr-argdecl-intro . 0)
|
| 688 | (label . -1000)
|
| 689 | (statement . 0)
|
| 690 | (statement-block-intro . +)
|
| 691 | (statement-case-intro . +)
|
| 692 | (statement-cont . +)
|
| 693 | (substatement . +)
|
| 694 | ))
|
| 695 | (indent-tabs-mode . t)
|
| 696 | (show-trailing-whitespace . t)
|
| 697 | ))))
|
| 698 |
|
| 699 | (dir-locals-set-directory-class
|
| 700 | (expand-file-name "~/src/linux-trees")
|
| 701 | 'linux-kernel)
|
| 702 |
|
| 703 | This will make emacs go better with the kernel coding style for C
|
| 704 | files below ``~/src/linux-trees``.
|
| 705 |
|
| 706 | But even if you fail in getting emacs to do sane formatting, not
|
| 707 | everything is lost: use ``indent``.
|
| 708 |
|
| 709 | Now, again, GNU indent has the same brain-dead settings that GNU emacs
|
| 710 | has, which is why you need to give it a few command line options.
|
| 711 | However, that's not too bad, because even the makers of GNU indent
|
| 712 | recognize the authority of K&R (the GNU people aren't evil, they are
|
| 713 | just severely misguided in this matter), so you just give indent the
|
| 714 | options ``-kr -i8`` (stands for ``K&R, 8 character indents``), or use
|
| 715 | ``scripts/Lindent``, which indents in the latest style.
|
| 716 |
|
| 717 | ``indent`` has a lot of options, and especially when it comes to comment
|
| 718 | re-formatting you may want to take a look at the man page. But
|
| 719 | remember: ``indent`` is not a fix for bad programming.
|
| 720 |
|
| 721 | Note that you can also use the ``clang-format`` tool to help you with
|
| 722 | these rules, to quickly re-format parts of your code automatically,
|
| 723 | and to review full files in order to spot coding style mistakes,
|
| 724 | typos and possible improvements. It is also handy for sorting ``#includes``,
|
| 725 | for aligning variables/macros, for reflowing text and other similar tasks.
|
| 726 | See the file :ref:`Documentation/dev-tools/clang-format.rst <clangformat>`
|
| 727 | for more details.
|
| 728 |
|
| 729 | Some basic editor settings, such as indentation and line endings, will be
|
| 730 | set automatically if you are using an editor that is compatible with
|
| 731 | EditorConfig. See the official EditorConfig website for more information:
|
| 732 | https://editorconfig.org/
|
| 733 |
|
| 734 | 10) Kconfig configuration files
|
| 735 | -------------------------------
|
| 736 |
|
| 737 | For all of the Kconfig* configuration files throughout the source tree,
|
| 738 | the indentation is somewhat different. Lines under a ``config`` definition
|
| 739 | are indented with one tab, while help text is indented an additional two
|
| 740 | spaces. Example::
|
| 741 |
|
| 742 | config AUDIT
|
| 743 | bool "Auditing support"
|
| 744 | depends on NET
|
| 745 | help
|
| 746 | Enable auditing infrastructure that can be used with another
|
| 747 | kernel subsystem, such as SELinux (which requires this for
|
| 748 | logging of avc messages output). Does not do system-call
|
| 749 | auditing without CONFIG_AUDITSYSCALL.
|
| 750 |
|
| 751 | Seriously dangerous features (such as write support for certain
|
| 752 | filesystems) should advertise this prominently in their prompt string::
|
| 753 |
|
| 754 | config ADFS_FS_RW
|
| 755 | bool "ADFS write support (DANGEROUS)"
|
| 756 | depends on ADFS_FS
|
| 757 | ...
|
| 758 |
|
| 759 | For full documentation on the configuration files, see the file
|
| 760 | Documentation/kbuild/kconfig-language.rst.
|
| 761 |
|
| 762 |
|
| 763 | 11) Data structures
|
| 764 | -------------------
|
| 765 |
|
| 766 | Data structures that have visibility outside the single-threaded
|
| 767 | environment they are created and destroyed in should always have
|
| 768 | reference counts. In the kernel, garbage collection doesn't exist (and
|
| 769 | outside the kernel garbage collection is slow and inefficient), which
|
| 770 | means that you absolutely **have** to reference count all your uses.
|
| 771 |
|
| 772 | Reference counting means that you can avoid locking, and allows multiple
|
| 773 | users to have access to the data structure in parallel - and not having
|
| 774 | to worry about the structure suddenly going away from under them just
|
| 775 | because they slept or did something else for a while.
|
| 776 |
|
| 777 | Note that locking is **not** a replacement for reference counting.
|
| 778 | Locking is used to keep data structures coherent, while reference
|
| 779 | counting is a memory management technique. Usually both are needed, and
|
| 780 | they are not to be confused with each other.
|
| 781 |
|
| 782 | Many data structures can indeed have two levels of reference counting,
|
| 783 | when there are users of different ``classes``. The subclass count counts
|
| 784 | the number of subclass users, and decrements the global count just once
|
| 785 | when the subclass count goes to zero.
|
| 786 |
|
| 787 | Examples of this kind of ``multi-level-reference-counting`` can be found in
|
| 788 | memory management (``struct mm_struct``: mm_users and mm_count), and in
|
| 789 | filesystem code (``struct super_block``: s_count and s_active).
|
| 790 |
|
| 791 | Remember: if another thread can find your data structure, and you don't
|
| 792 | have a reference count on it, you almost certainly have a bug.
|
| 793 |
|
| 794 |
|
| 795 | 12) Macros, Enums and RTL
|
| 796 | -------------------------
|
| 797 |
|
| 798 | Names of macros defining constants and labels in enums are capitalized.
|
| 799 |
|
| 800 | .. code-block:: c
|
| 801 |
|
| 802 | #define CONSTANT 0x12345
|
| 803 |
|
| 804 | Enums are preferred when defining several related constants.
|
| 805 |
|
| 806 | CAPITALIZED macro names are appreciated but macros resembling functions
|
| 807 | may be named in lower case.
|
| 808 |
|
| 809 | Generally, inline functions are preferable to macros resembling functions.
|
| 810 |
|
| 811 | Macros with multiple statements should be enclosed in a do - while block:
|
| 812 |
|
| 813 | .. code-block:: c
|
| 814 |
|
| 815 | #define macrofun(a, b, c) \
|
| 816 | do { \
|
| 817 | if (a == 5) \
|
| 818 | do_this(b, c); \
|
| 819 | } while (0)
|
| 820 |
|
| 821 | Function-like macros with unused parameters should be replaced by static
|
| 822 | inline functions to avoid the issue of unused variables:
|
| 823 |
|
| 824 | .. code-block:: c
|
| 825 |
|
| 826 | static inline void fun(struct foo *foo)
|
| 827 | {
|
| 828 | }
|
| 829 |
|
| 830 | Due to historical practices, many files still employ the "cast to (void)"
|
| 831 | approach to evaluate parameters. However, this method is not advisable.
|
| 832 | Inline functions address the issue of "expression with side effects
|
| 833 | evaluated more than once", circumvent unused-variable problems, and
|
| 834 | are generally better documented than macros for some reason.
|
| 835 |
|
| 836 | .. code-block:: c
|
| 837 |
|
| 838 | /*
|
| 839 | * Avoid doing this whenever possible and instead opt for static
|
| 840 | * inline functions
|
| 841 | */
|
| 842 | #define macrofun(foo) do { (void) (foo); } while (0)
|
| 843 |
|
| 844 | Things to avoid when using macros:
|
| 845 |
|
| 846 | 1) macros that affect control flow:
|
| 847 |
|
| 848 | .. code-block:: c
|
| 849 |
|
| 850 | #define FOO(x) \
|
| 851 | do { \
|
| 852 | if (blah(x) < 0) \
|
| 853 | return -EBUGGERED; \
|
| 854 | } while (0)
|
| 855 |
|
| 856 | is a **very** bad idea. It looks like a function call but exits the ``calling``
|
| 857 | function; don't break the internal parsers of those who will read the code.
|
| 858 |
|
| 859 | 2) macros that depend on having a local variable with a magic name:
|
| 860 |
|
| 861 | .. code-block:: c
|
| 862 |
|
| 863 | #define FOO(val) bar(index, val)
|
| 864 |
|
| 865 | might look like a good thing, but it's confusing as hell when one reads the
|
| 866 | code and it's prone to breakage from seemingly innocent changes.
|
| 867 |
|
| 868 | 3) macros with arguments that are used as l-values: FOO(x) = y; will
|
| 869 | bite you if somebody e.g. turns FOO into an inline function.
|
| 870 |
|
| 871 | 4) forgetting about precedence: macros defining constants using expressions
|
| 872 | must enclose the expression in parentheses. Beware of similar issues with
|
| 873 | macros using parameters.
|
| 874 |
|
| 875 | .. code-block:: c
|
| 876 |
|
| 877 | #define CONSTANT 0x4000
|
| 878 | #define CONSTEXP (CONSTANT | 3)
|
| 879 |
|
| 880 | 5) namespace collisions when defining local variables in macros resembling
|
| 881 | functions:
|
| 882 |
|
| 883 | .. code-block:: c
|
| 884 |
|
| 885 | #define FOO(x) \
|
| 886 | ({ \
|
| 887 | typeof(x) ret; \
|
| 888 | ret = calc_ret(x); \
|
| 889 | (ret); \
|
| 890 | })
|
| 891 |
|
| 892 | ret is a common name for a local variable - __foo_ret is less likely
|
| 893 | to collide with an existing variable.
|
| 894 |
|
| 895 | The cpp manual deals with macros exhaustively. The gcc internals manual also
|
| 896 | covers RTL which is used frequently with assembly language in the kernel.
|
| 897 |
|
| 898 |
|
| 899 | 13) Printing kernel messages
|
| 900 | ----------------------------
|
| 901 |
|
| 902 | Kernel developers like to be seen as literate. Do mind the spelling
|
| 903 | of kernel messages to make a good impression. Do not use incorrect
|
| 904 | contractions like ``dont``; use ``do not`` or ``don't`` instead. Make the
|
| 905 | messages concise, clear, and unambiguous.
|
| 906 |
|
| 907 | Kernel messages do not have to be terminated with a period.
|
| 908 |
|
| 909 | Printing numbers in parentheses (%d) adds no value and should be avoided.
|
| 910 |
|
| 911 | There are a number of driver model diagnostic macros in <linux/dev_printk.h>
|
| 912 | which you should use to make sure messages are matched to the right device
|
| 913 | and driver, and are tagged with the right level: dev_err(), dev_warn(),
|
| 914 | dev_info(), and so forth. For messages that aren't associated with a
|
| 915 | particular device, <linux/printk.h> defines pr_notice(), pr_info(),
|
| 916 | pr_warn(), pr_err(), etc. When drivers are working properly they are quiet,
|
| 917 | so prefer to use dev_dbg/pr_debug unless something is wrong.
|
| 918 |
|
| 919 | Coming up with good debugging messages can be quite a challenge; and once
|
| 920 | you have them, they can be a huge help for remote troubleshooting. However
|
| 921 | debug message printing is handled differently than printing other non-debug
|
| 922 | messages. While the other pr_XXX() functions print unconditionally,
|
| 923 | pr_debug() does not; it is compiled out by default, unless either DEBUG is
|
| 924 | defined or CONFIG_DYNAMIC_DEBUG is set. That is true for dev_dbg() also,
|
| 925 | and a related convention uses VERBOSE_DEBUG to add dev_vdbg() messages to
|
| 926 | the ones already enabled by DEBUG.
|
| 927 |
|
| 928 | Many subsystems have Kconfig debug options to turn on -DDEBUG in the
|
| 929 | corresponding Makefile; in other cases specific files #define DEBUG. And
|
| 930 | when a debug message should be unconditionally printed, such as if it is
|
| 931 | already inside a debug-related #ifdef section, printk(KERN_DEBUG ...) can be
|
| 932 | used.
|
| 933 |
|
| 934 |
|
| 935 | 14) Allocating memory
|
| 936 | ---------------------
|
| 937 |
|
| 938 | The kernel provides the following general purpose memory allocators:
|
| 939 | kmalloc(), kzalloc(), kmalloc_objs(), kzalloc_objs(), vmalloc(), and
|
| 940 | vzalloc(). Please refer to the API documentation for further information
|
| 941 | about them. :ref:`Documentation/core-api/memory-allocation.rst
|
| 942 | <memory_allocation>`
|
| 943 |
|
| 944 | The preferred form for passing a size of a struct is the following:
|
| 945 |
|
| 946 | .. code-block:: c
|
| 947 |
|
| 948 | p = kmalloc_obj(*p, ...);
|
| 949 |
|
| 950 | The alternative form where struct name is spelled out hurts readability and
|
| 951 | introduces an opportunity for a bug when the pointer variable type is changed
|
| 952 | but the corresponding sizeof that is passed to a memory allocator is not.
|
| 953 |
|
| 954 | Casting the return value which is a void pointer is redundant. The conversion
|
| 955 | from void pointer to any other pointer type is guaranteed by the C programming
|
| 956 | language.
|
| 957 |
|
| 958 | The preferred form for allocating an array is the following:
|
| 959 |
|
| 960 | .. code-block:: c
|
| 961 |
|
| 962 | p = kmalloc_objs(*p, n, ...);
|
| 963 |
|
| 964 | The preferred form for allocating a zeroed array is the following:
|
| 965 |
|
| 966 | .. code-block:: c
|
| 967 |
|
| 968 | p = kzalloc_objs(*p, n, ...);
|
| 969 |
|
| 970 | Both forms check for overflow on the allocation size n * sizeof(...),
|
| 971 | and return NULL if that occurred.
|
| 972 |
|
| 973 | These generic allocation functions all emit a stack dump on failure when used
|
| 974 | without __GFP_NOWARN so there is no use in emitting an additional failure
|
| 975 | message when NULL is returned.
|
| 976 |
|
| 977 | 15) The inline disease
|
| 978 | ----------------------
|
| 979 |
|
| 980 | There appears to be a common misperception that gcc has a magic "make me
|
| 981 | faster" speedup option called ``inline``. While the use of inlines can be
|
| 982 | appropriate (for example as a means of replacing macros, see Chapter 12), it
|
| 983 | very often is not. Abundant use of the inline keyword leads to a much bigger
|
| 984 | kernel, which in turn slows the system as a whole down, due to a bigger
|
| 985 | icache footprint for the CPU and simply because there is less memory
|
| 986 | available for the pagecache. Just think about it; a pagecache miss causes a
|
| 987 | disk seek, which easily takes 5 milliseconds. There are a LOT of cpu cycles
|
| 988 | that can go into these 5 milliseconds.
|
| 989 |
|
| 990 | A reasonable rule of thumb is to not put inline at functions that have more
|
| 991 | than 3 lines of code in them. An exception to this rule are the cases where
|
| 992 | a parameter is known to be a compile time constant, and as a result of this
|
| 993 | constantness you *know* the compiler will be able to optimize most of your
|
| 994 | function away at compile time. For a good example of this later case, see
|
| 995 | the kmalloc() inline function.
|
| 996 |
|
| 997 | Often people argue that adding inline to functions that are static and used
|
| 998 | only once is always a win since there is no space tradeoff. While this is
|
| 999 | technically correct, gcc is capable of inlining these automatically without
|
| 1000 | help, and the maintenance issue of removing the inline when a second user
|
| 1001 | appears outweighs the potential value of the hint that tells gcc to do
|
| 1002 | something it would have done anyway.
|
| 1003 |
|
| 1004 |
|
| 1005 | 16) Function return values and names
|
| 1006 | ------------------------------------
|
| 1007 |
|
| 1008 | Functions can return values of many different kinds, and one of the
|
| 1009 | most common is a value indicating whether the function succeeded or
|
| 1010 | failed. Such a value can be represented as an error-code integer
|
| 1011 | (-Exxx = failure, 0 = success) or a ``succeeded`` boolean (0 = failure,
|
| 1012 | non-zero = success).
|
| 1013 |
|
| 1014 | Mixing up these two sorts of representations is a fertile source of
|
| 1015 | difficult-to-find bugs. If the C language included a strong distinction
|
| 1016 | between integers and booleans then the compiler would find these mistakes
|
| 1017 | for us... but it doesn't. To help prevent such bugs, always follow this
|
| 1018 | convention::
|
| 1019 |
|
| 1020 | If the name of a function is an action or an imperative command,
|
| 1021 | the function should return an error-code integer. If the name
|
| 1022 | is a predicate, the function should return a "succeeded" boolean.
|
| 1023 |
|
| 1024 | For example, ``add work`` is a command, and the add_work() function returns 0
|
| 1025 | for success or -EBUSY for failure. In the same way, ``PCI device present`` is
|
| 1026 | a predicate, and the pci_dev_present() function returns 1 if it succeeds in
|
| 1027 | finding a matching device or 0 if it doesn't.
|
| 1028 |
|
| 1029 | All EXPORTed functions must respect this convention, and so should all
|
| 1030 | public functions. Private (static) functions need not, but it is
|
| 1031 | recommended that they do.
|
| 1032 |
|
| 1033 | Functions whose return value is the actual result of a computation, rather
|
| 1034 | than an indication of whether the computation succeeded, are not subject to
|
| 1035 | this rule. Generally they indicate failure by returning some out-of-range
|
| 1036 | result. Typical examples would be functions that return pointers; they use
|
| 1037 | NULL or the ERR_PTR mechanism to report failure.
|
| 1038 |
|
| 1039 |
|
| 1040 | 17) Using bool
|
| 1041 | --------------
|
| 1042 |
|
| 1043 | The Linux kernel bool type is an alias for the C99 _Bool type. bool values can
|
| 1044 | only evaluate to 0 or 1, and implicit or explicit conversion to bool
|
| 1045 | automatically converts the value to true or false. When using bool types the
|
| 1046 | !! construction is not needed, which eliminates a class of bugs.
|
| 1047 |
|
| 1048 | When working with bool values the true and false definitions should be used
|
| 1049 | instead of 1 and 0.
|
| 1050 |
|
| 1051 | bool function return types and stack variables are always fine to use whenever
|
| 1052 | appropriate. Use of bool is encouraged to improve readability and is often a
|
| 1053 | better option than 'int' for storing boolean values.
|
| 1054 |
|
| 1055 | Do not use bool if cache line layout or size of the value matters, as its size
|
| 1056 | and alignment varies based on the compiled architecture. Structures that are
|
| 1057 | optimized for alignment and size should not use bool.
|
| 1058 |
|
| 1059 | If a structure has many true/false values, consider consolidating them into a
|
| 1060 | bitfield with 1 bit members, or using an appropriate fixed width type, such as
|
| 1061 | u8.
|
| 1062 |
|
| 1063 | Similarly for function arguments, many true/false values can be consolidated
|
| 1064 | into a single bitwise 'flags' argument and 'flags' can often be a more
|
| 1065 | readable alternative if the call-sites have naked true/false constants.
|
| 1066 |
|
| 1067 | Otherwise limited use of bool in structures and arguments can improve
|
| 1068 | readability.
|
| 1069 |
|
| 1070 | 18) Don't re-invent the kernel macros
|
| 1071 | -------------------------------------
|
| 1072 |
|
| 1073 | There are many header files in include/linux/ that contain a number of macros that
|
| 1074 | you should use, rather than explicitly coding some variant of them yourself.
|
| 1075 | For example, if you need to calculate the length of an array, take advantage
|
| 1076 | of the macro
|
| 1077 |
|
| 1078 | .. code-block:: c
|
| 1079 |
|
| 1080 | #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
|
| 1081 |
|
| 1082 | which is defined in array_size.h.
|
| 1083 |
|
| 1084 | Similarly, if you need to calculate the size of some structure member, use
|
| 1085 |
|
| 1086 | .. code-block:: c
|
| 1087 |
|
| 1088 | #define sizeof_field(t, f) (sizeof(((t*)0)->f))
|
| 1089 |
|
| 1090 | which is defined in stddef.h.
|
| 1091 |
|
| 1092 | There are also min() and max() macros defined in minmax.h that do strict type checking
|
| 1093 | if you need them. Feel free to peruse the header files to see what else is already
|
| 1094 | defined that you shouldn't reproduce in your code.
|
| 1095 |
|
| 1096 |
|
| 1097 | 19) Editor modelines and other cruft
|
| 1098 | ------------------------------------
|
| 1099 |
|
| 1100 | Some editors can interpret configuration information embedded in source files,
|
| 1101 | indicated with special markers. For example, emacs interprets lines marked
|
| 1102 | like this:
|
| 1103 |
|
| 1104 | .. code-block:: c
|
| 1105 |
|
| 1106 | -*- mode: c -*-
|
| 1107 |
|
| 1108 | Or like this:
|
| 1109 |
|
| 1110 | .. code-block:: c
|
| 1111 |
|
| 1112 | /*
|
| 1113 | Local Variables:
|
| 1114 | compile-command: "gcc -DMAGIC_DEBUG_FLAG foo.c"
|
| 1115 | End:
|
| 1116 | */
|
| 1117 |
|
| 1118 | Vim interprets markers that look like this:
|
| 1119 |
|
| 1120 | .. code-block:: c
|
| 1121 |
|
| 1122 | /* vim:set sw=8 noet */
|
| 1123 |
|
| 1124 | Do not include any of these in source files. People have their own personal
|
| 1125 | editor configurations, and your source files should not override them. This
|
| 1126 | includes markers for indentation and mode configuration. People may use their
|
| 1127 | own custom mode, or may have some other magic method for making indentation
|
| 1128 | work correctly.
|
| 1129 |
|
| 1130 |
|
| 1131 | 20) Inline assembly
|
| 1132 | -------------------
|
| 1133 |
|
| 1134 | In architecture-specific code, you may need to use inline assembly to interface
|
| 1135 | with CPU or platform functionality. Don't hesitate to do so when necessary.
|
| 1136 | However, don't use inline assembly gratuitously when C can do the job. You can
|
| 1137 | and should poke hardware from C when possible.
|
| 1138 |
|
| 1139 | Consider writing simple helper functions that wrap common bits of inline
|
| 1140 | assembly, rather than repeatedly writing them with slight variations. Remember
|
| 1141 | that inline assembly can use C parameters.
|
| 1142 |
|
| 1143 | Large, non-trivial assembly functions should go in .S files, with corresponding
|
| 1144 | C prototypes defined in C header files. The C prototypes for assembly
|
| 1145 | functions should use ``asmlinkage``.
|
| 1146 |
|
| 1147 | You may need to mark your asm statement as volatile, to prevent GCC from
|
| 1148 | removing it if GCC doesn't notice any side effects. You don't always need to
|
| 1149 | do so, though, and doing so unnecessarily can limit optimization.
|
| 1150 |
|
| 1151 | When writing a single inline assembly statement containing multiple
|
| 1152 | instructions, put each instruction on a separate line in a separate quoted
|
| 1153 | string, and end each string except the last with ``\n\t`` to properly indent
|
| 1154 | the next instruction in the assembly output:
|
| 1155 |
|
| 1156 | .. code-block:: c
|
| 1157 |
|
| 1158 | asm ("magic %reg1, #42\n\t"
|
| 1159 | "more_magic %reg2, %reg3"
|
| 1160 | : /* outputs */ : /* inputs */ : /* clobbers */);
|
| 1161 |
|
| 1162 |
|
| 1163 | 21) Conditional Compilation
|
| 1164 | ---------------------------
|
| 1165 |
|
| 1166 | Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c
|
| 1167 | files; doing so makes code harder to read and logic harder to follow. Instead,
|
| 1168 | use such conditionals in a header file defining functions for use in those .c
|
| 1169 | files, providing no-op stub versions in the #else case, and then call those
|
| 1170 | functions unconditionally from .c files. The compiler will avoid generating
|
| 1171 | any code for the stub calls, producing identical results, but the logic will
|
| 1172 | remain easy to follow.
|
| 1173 |
|
| 1174 | Prefer to compile out entire functions, rather than portions of functions or
|
| 1175 | portions of expressions. Rather than putting an ifdef in an expression, factor
|
| 1176 | out part or all of the expression into a separate helper function and apply the
|
| 1177 | conditional to that function.
|
| 1178 |
|
| 1179 | If you have a function or variable which may potentially go unused in a
|
| 1180 | particular configuration, and the compiler would warn about its definition
|
| 1181 | going unused, mark the definition as __maybe_unused rather than wrapping it in
|
| 1182 | a preprocessor conditional. (However, if a function or variable *always* goes
|
| 1183 | unused, delete it.)
|
| 1184 |
|
| 1185 | Within code, where possible, use the IS_ENABLED macro to convert a Kconfig
|
| 1186 | symbol into a C boolean expression, and use it in a normal C conditional:
|
| 1187 |
|
| 1188 | .. code-block:: c
|
| 1189 |
|
| 1190 | if (IS_ENABLED(CONFIG_SOMETHING)) {
|
| 1191 | ...
|
| 1192 | }
|
| 1193 |
|
| 1194 | The compiler will constant-fold the conditional away, and include or exclude
|
| 1195 | the block of code just as with an #ifdef, so this will not add any runtime
|
| 1196 | overhead. However, this approach still allows the C compiler to see the code
|
| 1197 | inside the block, and check it for correctness (syntax, types, symbol
|
| 1198 | references, etc). Thus, you still have to use an #ifdef if the code inside the
|
| 1199 | block references symbols that will not exist if the condition is not met.
|
| 1200 |
|
| 1201 | At the end of any non-trivial #if or #ifdef block (more than a few lines),
|
| 1202 | place a comment after the #endif on the same line, noting the conditional
|
| 1203 | expression used. For instance:
|
| 1204 |
|
| 1205 | .. code-block:: c
|
| 1206 |
|
| 1207 | #ifdef CONFIG_SOMETHING
|
| 1208 | ...
|
| 1209 | #endif /* CONFIG_SOMETHING */
|
| 1210 |
|
| 1211 |
|
| 1212 | 22) Do not crash the kernel
|
| 1213 | ---------------------------
|
| 1214 |
|
| 1215 | In general, the decision to crash the kernel belongs to the user, rather
|
| 1216 | than to the kernel developer.
|
| 1217 |
|
| 1218 | Avoid panic()
|
| 1219 | *************
|
| 1220 |
|
| 1221 | panic() should be used with care and primarily only during system boot.
|
| 1222 | panic() is, for example, acceptable when running out of memory during boot and
|
| 1223 | not being able to continue.
|
| 1224 |
|
| 1225 | Use WARN() rather than BUG()
|
| 1226 | ****************************
|
| 1227 |
|
| 1228 | Do not add new code that uses any of the BUG() variants, such as BUG(),
|
| 1229 | BUG_ON(), or VM_BUG_ON(). Instead, use a WARN*() variant, preferably
|
| 1230 | WARN_ON_ONCE(), and possibly with recovery code. Recovery code is not
|
| 1231 | required if there is no reasonable way to at least partially recover.
|
| 1232 |
|
| 1233 | "I'm too lazy to do error handling" is not an excuse for using BUG(). Major
|
| 1234 | internal corruptions with no way of continuing may still use BUG(), but need
|
| 1235 | good justification.
|
| 1236 |
|
| 1237 | Use WARN_ON_ONCE() rather than WARN() or WARN_ON()
|
| 1238 | **************************************************
|
| 1239 |
|
| 1240 | WARN_ON_ONCE() is generally preferred over WARN() or WARN_ON(), because it
|
| 1241 | is common for a given warning condition, if it occurs at all, to occur
|
| 1242 | multiple times. This can fill up and wrap the kernel log, and can even slow
|
| 1243 | the system enough that the excessive logging turns into its own, additional
|
| 1244 | problem.
|
| 1245 |
|
| 1246 | Do not WARN lightly
|
| 1247 | *******************
|
| 1248 |
|
| 1249 | WARN*() is intended for unexpected, this-should-never-happen situations.
|
| 1250 | WARN*() macros are not to be used for anything that is expected to happen
|
| 1251 | during normal operation. These are not pre- or post-condition asserts, for
|
| 1252 | example. Again: WARN*() must not be used for a condition that is expected
|
| 1253 | to trigger easily, for example, by user space actions. pr_warn_once() is a
|
| 1254 | possible alternative, if you need to notify the user of a problem.
|
| 1255 |
|
| 1256 | Do not worry about panic_on_warn users
|
| 1257 | **************************************
|
| 1258 |
|
| 1259 | A few more words about panic_on_warn: Remember that ``panic_on_warn`` is an
|
| 1260 | available kernel option, and that many users set this option. This is why
|
| 1261 | there is a "Do not WARN lightly" writeup, above. However, the existence of
|
| 1262 | panic_on_warn users is not a valid reason to avoid the judicious use
|
| 1263 | WARN*(). That is because, whoever enables panic_on_warn has explicitly
|
| 1264 | asked the kernel to crash if a WARN*() fires, and such users must be
|
| 1265 | prepared to deal with the consequences of a system that is somewhat more
|
| 1266 | likely to crash.
|
| 1267 |
|
| 1268 | Use BUILD_BUG_ON() for compile-time assertions
|
| 1269 | **********************************************
|
| 1270 |
|
| 1271 | The use of BUILD_BUG_ON() is acceptable and encouraged, because it is a
|
| 1272 | compile-time assertion that has no effect at runtime.
|
| 1273 |
|
| 1274 | Appendix I) References
|
| 1275 | ----------------------
|
| 1276 |
|
| 1277 | The C Programming Language, Second Edition
|
| 1278 | by Brian W. Kernighan and Dennis M. Ritchie.
|
| 1279 | Prentice Hall, Inc., 1988.
|
| 1280 | ISBN 0-13-110362-8 (paperback), 0-13-110370-9 (hardback).
|
| 1281 |
|
| 1282 | The Practice of Programming
|
| 1283 | by Brian W. Kernighan and Rob Pike.
|
| 1284 | Addison-Wesley, Inc., 1999.
|
| 1285 | ISBN 0-201-61586-X.
|
| 1286 |
|
| 1287 | GNU manuals - where in compliance with K&R and this text - for cpp, gcc,
|
| 1288 | gcc internals and indent, all available from https://www.gnu.org/manual/
|
| 1289 |
|
| 1290 | WG14 is the international standardization working group for the programming
|
| 1291 | language C, URL: http://www.open-std.org/JTC1/SC22/WG14/
|
| 1292 |
|
| 1293 | Kernel CodingStyle, by [email protected] at OLS 2002:
|
| 1294 | http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
|