1186128f7SMauro Carvalho Chehab.. _codingstyle: 2186128f7SMauro Carvalho Chehab 3186128f7SMauro Carvalho ChehabLinux kernel coding style 4186128f7SMauro Carvalho Chehab========================= 5186128f7SMauro Carvalho Chehab 6186128f7SMauro Carvalho ChehabThis is a short document describing the preferred coding style for the 7186128f7SMauro Carvalho Chehablinux kernel. Coding style is very personal, and I won't **force** my 8186128f7SMauro Carvalho Chehabviews on anybody, but this is what goes for anything that I have to be 9186128f7SMauro Carvalho Chehabable to maintain, and I'd prefer it for most other things too. Please 10186128f7SMauro Carvalho Chehabat least consider the points made here. 11186128f7SMauro Carvalho Chehab 12186128f7SMauro Carvalho ChehabFirst off, I'd suggest printing out a copy of the GNU coding standards, 13186128f7SMauro Carvalho Chehaband NOT read it. Burn them, it's a great symbolic gesture. 14186128f7SMauro Carvalho Chehab 15186128f7SMauro Carvalho ChehabAnyway, here goes: 16186128f7SMauro Carvalho Chehab 17186128f7SMauro Carvalho Chehab 18186128f7SMauro Carvalho Chehab1) Indentation 19186128f7SMauro Carvalho Chehab-------------- 20186128f7SMauro Carvalho Chehab 21186128f7SMauro Carvalho ChehabTabs are 8 characters, and thus indentations are also 8 characters. 22186128f7SMauro Carvalho ChehabThere are heretic movements that try to make indentations 4 (or even 2!) 23186128f7SMauro Carvalho Chehabcharacters deep, and that is akin to trying to define the value of PI to 24186128f7SMauro Carvalho Chehabbe 3. 25186128f7SMauro Carvalho Chehab 26186128f7SMauro Carvalho ChehabRationale: The whole idea behind indentation is to clearly define where 27186128f7SMauro Carvalho Chehaba block of control starts and ends. Especially when you've been looking 28186128f7SMauro Carvalho Chehabat your screen for 20 straight hours, you'll find it a lot easier to see 29186128f7SMauro Carvalho Chehabhow the indentation works if you have large indentations. 30186128f7SMauro Carvalho Chehab 31186128f7SMauro Carvalho ChehabNow, some people will claim that having 8-character indentations makes 32186128f7SMauro Carvalho Chehabthe code move too far to the right, and makes it hard to read on a 33186128f7SMauro Carvalho Chehab80-character terminal screen. The answer to that is that if you need 34186128f7SMauro Carvalho Chehabmore than 3 levels of indentation, you're screwed anyway, and should fix 35186128f7SMauro Carvalho Chehabyour program. 36186128f7SMauro Carvalho Chehab 37186128f7SMauro Carvalho ChehabIn short, 8-char indents make things easier to read, and have the added 38186128f7SMauro Carvalho Chehabbenefit of warning you when you're nesting your functions too deep. 39186128f7SMauro Carvalho ChehabHeed that warning. 40186128f7SMauro Carvalho Chehab 41186128f7SMauro Carvalho ChehabThe preferred way to ease multiple indentation levels in a switch statement is 42186128f7SMauro Carvalho Chehabto align the ``switch`` and its subordinate ``case`` labels in the same column 43186128f7SMauro Carvalho Chehabinstead of ``double-indenting`` the ``case`` labels. E.g.: 44186128f7SMauro Carvalho Chehab 45186128f7SMauro Carvalho Chehab.. code-block:: c 46186128f7SMauro Carvalho Chehab 47186128f7SMauro Carvalho Chehab switch (suffix) { 48186128f7SMauro Carvalho Chehab case 'G': 49186128f7SMauro Carvalho Chehab case 'g': 50186128f7SMauro Carvalho Chehab mem <<= 30; 51186128f7SMauro Carvalho Chehab break; 52186128f7SMauro Carvalho Chehab case 'M': 53186128f7SMauro Carvalho Chehab case 'm': 54186128f7SMauro Carvalho Chehab mem <<= 20; 55186128f7SMauro Carvalho Chehab break; 56186128f7SMauro Carvalho Chehab case 'K': 57186128f7SMauro Carvalho Chehab case 'k': 58186128f7SMauro Carvalho Chehab mem <<= 10; 59b9918bdcSJoe Perches fallthrough; 60186128f7SMauro Carvalho Chehab default: 61186128f7SMauro Carvalho Chehab break; 62186128f7SMauro Carvalho Chehab } 63186128f7SMauro Carvalho Chehab 64186128f7SMauro Carvalho ChehabDon't put multiple statements on a single line unless you have 65186128f7SMauro Carvalho Chehabsomething to hide: 66186128f7SMauro Carvalho Chehab 67186128f7SMauro Carvalho Chehab.. code-block:: c 68186128f7SMauro Carvalho Chehab 69186128f7SMauro Carvalho Chehab if (condition) do_this; 70186128f7SMauro Carvalho Chehab do_something_everytime; 71186128f7SMauro Carvalho Chehab 7226606ce0SJoe PerchesDon't use commas to avoid using braces: 7326606ce0SJoe Perches 7426606ce0SJoe Perches.. code-block:: c 7526606ce0SJoe Perches 7626606ce0SJoe Perches if (condition) 7726606ce0SJoe Perches do_this(), do_that(); 7826606ce0SJoe Perches 7926606ce0SJoe PerchesAlways uses braces for multiple statements: 8026606ce0SJoe Perches 8126606ce0SJoe Perches.. code-block:: c 8226606ce0SJoe Perches 8326606ce0SJoe Perches if (condition) { 8426606ce0SJoe Perches do_this(); 8526606ce0SJoe Perches do_that(); 8626606ce0SJoe Perches } 8726606ce0SJoe Perches 88186128f7SMauro Carvalho ChehabDon't put multiple assignments on a single line either. Kernel coding style 89186128f7SMauro Carvalho Chehabis super simple. Avoid tricky expressions. 90186128f7SMauro Carvalho Chehab 9126606ce0SJoe Perches 92186128f7SMauro Carvalho ChehabOutside of comments, documentation and except in Kconfig, spaces are never 93186128f7SMauro Carvalho Chehabused for indentation, and the above example is deliberately broken. 94186128f7SMauro Carvalho Chehab 95186128f7SMauro Carvalho ChehabGet a decent editor and don't leave whitespace at the end of lines. 96186128f7SMauro Carvalho Chehab 97186128f7SMauro Carvalho Chehab 98186128f7SMauro Carvalho Chehab2) Breaking long lines and strings 99186128f7SMauro Carvalho Chehab---------------------------------- 100186128f7SMauro Carvalho Chehab 101186128f7SMauro Carvalho ChehabCoding style is all about readability and maintainability using commonly 102186128f7SMauro Carvalho Chehabavailable tools. 103186128f7SMauro Carvalho Chehab 104bdc48fa1SJoe PerchesThe preferred limit on the length of a single line is 80 columns. 105186128f7SMauro Carvalho Chehab 106bdc48fa1SJoe PerchesStatements longer than 80 columns should be broken into sensible chunks, 107bdc48fa1SJoe Perchesunless exceeding 80 columns significantly increases readability and does 108bdc48fa1SJoe Perchesnot hide information. 109bdc48fa1SJoe Perches 11077d22a43SGeert UytterhoevenDescendants are always substantially shorter than the parent and 111bdc48fa1SJoe Perchesare placed substantially to the right. A very commonly used style 112bdc48fa1SJoe Perchesis to align descendants to a function open parenthesis. 113bdc48fa1SJoe Perches 114bdc48fa1SJoe PerchesThese same rules are applied to function headers with a long argument list. 115bdc48fa1SJoe Perches 116bdc48fa1SJoe PerchesHowever, never break user-visible strings such as printk messages because 117bdc48fa1SJoe Perchesthat breaks the ability to grep for them. 118186128f7SMauro Carvalho Chehab 119186128f7SMauro Carvalho Chehab 120186128f7SMauro Carvalho Chehab3) Placing Braces and Spaces 121186128f7SMauro Carvalho Chehab---------------------------- 122186128f7SMauro Carvalho Chehab 123186128f7SMauro Carvalho ChehabThe other issue that always comes up in C styling is the placement of 124186128f7SMauro Carvalho Chehabbraces. Unlike the indent size, there are few technical reasons to 125186128f7SMauro Carvalho Chehabchoose one placement strategy over the other, but the preferred way, as 126186128f7SMauro Carvalho Chehabshown to us by the prophets Kernighan and Ritchie, is to put the opening 127186128f7SMauro Carvalho Chehabbrace last on the line, and put the closing brace first, thusly: 128186128f7SMauro Carvalho Chehab 129186128f7SMauro Carvalho Chehab.. code-block:: c 130186128f7SMauro Carvalho Chehab 131186128f7SMauro Carvalho Chehab if (x is true) { 132186128f7SMauro Carvalho Chehab we do y 133186128f7SMauro Carvalho Chehab } 134186128f7SMauro Carvalho Chehab 135186128f7SMauro Carvalho ChehabThis applies to all non-function statement blocks (if, switch, for, 136186128f7SMauro Carvalho Chehabwhile, do). E.g.: 137186128f7SMauro Carvalho Chehab 138186128f7SMauro Carvalho Chehab.. code-block:: c 139186128f7SMauro Carvalho Chehab 140186128f7SMauro Carvalho Chehab switch (action) { 141186128f7SMauro Carvalho Chehab case KOBJ_ADD: 142186128f7SMauro Carvalho Chehab return "add"; 143186128f7SMauro Carvalho Chehab case KOBJ_REMOVE: 144186128f7SMauro Carvalho Chehab return "remove"; 145186128f7SMauro Carvalho Chehab case KOBJ_CHANGE: 146186128f7SMauro Carvalho Chehab return "change"; 147186128f7SMauro Carvalho Chehab default: 148186128f7SMauro Carvalho Chehab return NULL; 149186128f7SMauro Carvalho Chehab } 150186128f7SMauro Carvalho Chehab 151186128f7SMauro Carvalho ChehabHowever, there is one special case, namely functions: they have the 152186128f7SMauro Carvalho Chehabopening brace at the beginning of the next line, thus: 153186128f7SMauro Carvalho Chehab 154186128f7SMauro Carvalho Chehab.. code-block:: c 155186128f7SMauro Carvalho Chehab 156186128f7SMauro Carvalho Chehab int function(int x) 157186128f7SMauro Carvalho Chehab { 158186128f7SMauro Carvalho Chehab body of function 159186128f7SMauro Carvalho Chehab } 160186128f7SMauro Carvalho Chehab 161186128f7SMauro Carvalho ChehabHeretic people all over the world have claimed that this inconsistency 162186128f7SMauro Carvalho Chehabis ... well ... inconsistent, but all right-thinking people know that 163186128f7SMauro Carvalho Chehab(a) K&R are **right** and (b) K&R are right. Besides, functions are 164186128f7SMauro Carvalho Chehabspecial anyway (you can't nest them in C). 165186128f7SMauro Carvalho Chehab 166186128f7SMauro Carvalho ChehabNote that the closing brace is empty on a line of its own, **except** in 167186128f7SMauro Carvalho Chehabthe cases where it is followed by a continuation of the same statement, 168186128f7SMauro Carvalho Chehabie a ``while`` in a do-statement or an ``else`` in an if-statement, like 169186128f7SMauro Carvalho Chehabthis: 170186128f7SMauro Carvalho Chehab 171186128f7SMauro Carvalho Chehab.. code-block:: c 172186128f7SMauro Carvalho Chehab 173186128f7SMauro Carvalho Chehab do { 174186128f7SMauro Carvalho Chehab body of do-loop 175186128f7SMauro Carvalho Chehab } while (condition); 176186128f7SMauro Carvalho Chehab 177186128f7SMauro Carvalho Chehaband 178186128f7SMauro Carvalho Chehab 179186128f7SMauro Carvalho Chehab.. code-block:: c 180186128f7SMauro Carvalho Chehab 181186128f7SMauro Carvalho Chehab if (x == y) { 182186128f7SMauro Carvalho Chehab .. 183186128f7SMauro Carvalho Chehab } else if (x > y) { 184186128f7SMauro Carvalho Chehab ... 185186128f7SMauro Carvalho Chehab } else { 186186128f7SMauro Carvalho Chehab .... 187186128f7SMauro Carvalho Chehab } 188186128f7SMauro Carvalho Chehab 189186128f7SMauro Carvalho ChehabRationale: K&R. 190186128f7SMauro Carvalho Chehab 191186128f7SMauro Carvalho ChehabAlso, note that this brace-placement also minimizes the number of empty 192186128f7SMauro Carvalho Chehab(or almost empty) lines, without any loss of readability. Thus, as the 193186128f7SMauro Carvalho Chehabsupply of new-lines on your screen is not a renewable resource (think 194186128f7SMauro Carvalho Chehab25-line terminal screens here), you have more empty lines to put 195186128f7SMauro Carvalho Chehabcomments on. 196186128f7SMauro Carvalho Chehab 197186128f7SMauro Carvalho ChehabDo not unnecessarily use braces where a single statement will do. 198186128f7SMauro Carvalho Chehab 199186128f7SMauro Carvalho Chehab.. code-block:: c 200186128f7SMauro Carvalho Chehab 201186128f7SMauro Carvalho Chehab if (condition) 202186128f7SMauro Carvalho Chehab action(); 203186128f7SMauro Carvalho Chehab 204186128f7SMauro Carvalho Chehaband 205186128f7SMauro Carvalho Chehab 206185ea767SThorsten Blum.. code-block:: c 207186128f7SMauro Carvalho Chehab 208186128f7SMauro Carvalho Chehab if (condition) 209186128f7SMauro Carvalho Chehab do_this(); 210186128f7SMauro Carvalho Chehab else 211186128f7SMauro Carvalho Chehab do_that(); 212186128f7SMauro Carvalho Chehab 213186128f7SMauro Carvalho ChehabThis does not apply if only one branch of a conditional statement is a single 214186128f7SMauro Carvalho Chehabstatement; in the latter case use braces in both branches: 215186128f7SMauro Carvalho Chehab 216186128f7SMauro Carvalho Chehab.. code-block:: c 217186128f7SMauro Carvalho Chehab 218186128f7SMauro Carvalho Chehab if (condition) { 219186128f7SMauro Carvalho Chehab do_this(); 220186128f7SMauro Carvalho Chehab do_that(); 221186128f7SMauro Carvalho Chehab } else { 222186128f7SMauro Carvalho Chehab otherwise(); 223186128f7SMauro Carvalho Chehab } 224186128f7SMauro Carvalho Chehab 2251dbba2cfSGary R HookAlso, use braces when a loop contains more than a single simple statement: 2261dbba2cfSGary R Hook 2271dbba2cfSGary R Hook.. code-block:: c 2281dbba2cfSGary R Hook 2291dbba2cfSGary R Hook while (condition) { 2301dbba2cfSGary R Hook if (test) 2311dbba2cfSGary R Hook do_something(); 2321dbba2cfSGary R Hook } 2331dbba2cfSGary R Hook 234186128f7SMauro Carvalho Chehab3.1) Spaces 235186128f7SMauro Carvalho Chehab*********** 236186128f7SMauro Carvalho Chehab 237186128f7SMauro Carvalho ChehabLinux kernel style for use of spaces depends (mostly) on 238186128f7SMauro Carvalho Chehabfunction-versus-keyword usage. Use a space after (most) keywords. The 239186128f7SMauro Carvalho Chehabnotable exceptions are sizeof, typeof, alignof, and __attribute__, which look 240186128f7SMauro Carvalho Chehabsomewhat like functions (and are usually used with parentheses in Linux, 241186128f7SMauro Carvalho Chehabalthough they are not required in the language, as in: ``sizeof info`` after 242186128f7SMauro Carvalho Chehab``struct fileinfo info;`` is declared). 243186128f7SMauro Carvalho Chehab 244186128f7SMauro Carvalho ChehabSo use a space after these keywords:: 245186128f7SMauro Carvalho Chehab 246186128f7SMauro Carvalho Chehab if, switch, case, for, do, while 247186128f7SMauro Carvalho Chehab 248186128f7SMauro Carvalho Chehabbut not with sizeof, typeof, alignof, or __attribute__. E.g., 249186128f7SMauro Carvalho Chehab 250186128f7SMauro Carvalho Chehab.. code-block:: c 251186128f7SMauro Carvalho Chehab 252186128f7SMauro Carvalho Chehab 253186128f7SMauro Carvalho Chehab s = sizeof(struct file); 254186128f7SMauro Carvalho Chehab 255186128f7SMauro Carvalho ChehabDo not add spaces around (inside) parenthesized expressions. This example is 256186128f7SMauro Carvalho Chehab**bad**: 257186128f7SMauro Carvalho Chehab 258186128f7SMauro Carvalho Chehab.. code-block:: c 259186128f7SMauro Carvalho Chehab 260186128f7SMauro Carvalho Chehab 261186128f7SMauro Carvalho Chehab s = sizeof( struct file ); 262186128f7SMauro Carvalho Chehab 263186128f7SMauro Carvalho ChehabWhen declaring pointer data or a function that returns a pointer type, the 264186128f7SMauro Carvalho Chehabpreferred use of ``*`` is adjacent to the data name or function name and not 265186128f7SMauro Carvalho Chehabadjacent to the type name. Examples: 266186128f7SMauro Carvalho Chehab 267186128f7SMauro Carvalho Chehab.. code-block:: c 268186128f7SMauro Carvalho Chehab 269186128f7SMauro Carvalho Chehab 270186128f7SMauro Carvalho Chehab char *linux_banner; 271186128f7SMauro Carvalho Chehab unsigned long long memparse(char *ptr, char **retptr); 272186128f7SMauro Carvalho Chehab char *match_strdup(substring_t *s); 273186128f7SMauro Carvalho Chehab 274186128f7SMauro Carvalho ChehabUse one space around (on each side of) most binary and ternary operators, 275186128f7SMauro Carvalho Chehabsuch as any of these:: 276186128f7SMauro Carvalho Chehab 277186128f7SMauro Carvalho Chehab = + - < > * / % | & ^ <= >= == != ? : 278186128f7SMauro Carvalho Chehab 279186128f7SMauro Carvalho Chehabbut no space after unary operators:: 280186128f7SMauro Carvalho Chehab 281186128f7SMauro Carvalho Chehab & * + - ~ ! sizeof typeof alignof __attribute__ defined 282186128f7SMauro Carvalho Chehab 283186128f7SMauro Carvalho Chehabno space before the postfix increment & decrement unary operators:: 284186128f7SMauro Carvalho Chehab 285186128f7SMauro Carvalho Chehab ++ -- 286186128f7SMauro Carvalho Chehab 287186128f7SMauro Carvalho Chehabno space after the prefix increment & decrement unary operators:: 288186128f7SMauro Carvalho Chehab 289186128f7SMauro Carvalho Chehab ++ -- 290186128f7SMauro Carvalho Chehab 291186128f7SMauro Carvalho Chehaband no space around the ``.`` and ``->`` structure member operators. 292186128f7SMauro Carvalho Chehab 293186128f7SMauro Carvalho ChehabDo not leave trailing whitespace at the ends of lines. Some editors with 294186128f7SMauro Carvalho Chehab``smart`` indentation will insert whitespace at the beginning of new lines as 295186128f7SMauro Carvalho Chehabappropriate, so you can start typing the next line of code right away. 296186128f7SMauro Carvalho ChehabHowever, some such editors do not remove the whitespace if you end up not 297186128f7SMauro Carvalho Chehabputting a line of code there, such as if you leave a blank line. As a result, 298186128f7SMauro Carvalho Chehabyou end up with lines containing trailing whitespace. 299186128f7SMauro Carvalho Chehab 300186128f7SMauro Carvalho ChehabGit will warn you about patches that introduce trailing whitespace, and can 301186128f7SMauro Carvalho Chehaboptionally strip the trailing whitespace for you; however, if applying a series 302186128f7SMauro Carvalho Chehabof patches, this may make later patches in the series fail by changing their 303186128f7SMauro Carvalho Chehabcontext lines. 304186128f7SMauro Carvalho Chehab 305186128f7SMauro Carvalho Chehab 306186128f7SMauro Carvalho Chehab4) Naming 307186128f7SMauro Carvalho Chehab--------- 308186128f7SMauro Carvalho Chehab 309fb0e0ffeSTony FischettiC is a Spartan language, and your naming conventions should follow suit. 310fb0e0ffeSTony FischettiUnlike Modula-2 and Pascal programmers, C programmers do not use cute 311fb0e0ffeSTony Fischettinames like ThisVariableIsATemporaryCounter. A C programmer would call that 312186128f7SMauro Carvalho Chehabvariable ``tmp``, which is much easier to write, and not the least more 313186128f7SMauro Carvalho Chehabdifficult to understand. 314186128f7SMauro Carvalho Chehab 315186128f7SMauro Carvalho ChehabHOWEVER, while mixed-case names are frowned upon, descriptive names for 316186128f7SMauro Carvalho Chehabglobal variables are a must. To call a global function ``foo`` is a 317186128f7SMauro Carvalho Chehabshooting offense. 318186128f7SMauro Carvalho Chehab 319186128f7SMauro Carvalho ChehabGLOBAL variables (to be used only if you **really** need them) need to 320186128f7SMauro Carvalho Chehabhave descriptive names, as do global functions. If you have a function 321186128f7SMauro Carvalho Chehabthat counts the number of active users, you should call that 322186128f7SMauro Carvalho Chehab``count_active_users()`` or similar, you should **not** call it ``cntusr()``. 323186128f7SMauro Carvalho Chehab 324186128f7SMauro Carvalho ChehabEncoding the type of a function into the name (so-called Hungarian 325fb0e0ffeSTony Fischettinotation) is asinine - the compiler knows the types anyway and can check 326b7592e5bSYorick de Widthose, and it only confuses the programmer. 327186128f7SMauro Carvalho Chehab 328186128f7SMauro Carvalho ChehabLOCAL variable names should be short, and to the point. If you have 329186128f7SMauro Carvalho Chehabsome random integer loop counter, it should probably be called ``i``. 330186128f7SMauro Carvalho ChehabCalling it ``loop_counter`` is non-productive, if there is no chance of it 331186128f7SMauro Carvalho Chehabbeing mis-understood. Similarly, ``tmp`` can be just about any type of 332186128f7SMauro Carvalho Chehabvariable that is used to hold a temporary value. 333186128f7SMauro Carvalho Chehab 334186128f7SMauro Carvalho ChehabIf you are afraid to mix up your local variable names, you have another 335186128f7SMauro Carvalho Chehabproblem, which is called the function-growth-hormone-imbalance syndrome. 336186128f7SMauro Carvalho ChehabSee chapter 6 (Functions). 337186128f7SMauro Carvalho Chehab 338a5f526ecSDan WilliamsFor symbol names and documentation, avoid introducing new usage of 339a5f526ecSDan Williams'master / slave' (or 'slave' independent of 'master') and 'blacklist / 340a5f526ecSDan Williamswhitelist'. 341a5f526ecSDan Williams 342a5f526ecSDan WilliamsRecommended replacements for 'master / slave' are: 343a5f526ecSDan Williams '{primary,main} / {secondary,replica,subordinate}' 344a5f526ecSDan Williams '{initiator,requester} / {target,responder}' 345a5f526ecSDan Williams '{controller,host} / {device,worker,proxy}' 346a5f526ecSDan Williams 'leader / follower' 347a5f526ecSDan Williams 'director / performer' 348a5f526ecSDan Williams 349a5f526ecSDan WilliamsRecommended replacements for 'blacklist/whitelist' are: 350a5f526ecSDan Williams 'denylist / allowlist' 351a5f526ecSDan Williams 'blocklist / passlist' 352a5f526ecSDan Williams 353a5f526ecSDan WilliamsExceptions for introducing new usage is to maintain a userspace ABI/API, 354a5f526ecSDan Williamsor when updating code for an existing (as of 2020) hardware or protocol 355a5f526ecSDan Williamsspecification that mandates those terms. For new specifications 356a5f526ecSDan Williamstranslate specification usage of the terminology to the kernel coding 357a5f526ecSDan Williamsstandard where possible. 358186128f7SMauro Carvalho Chehab 359186128f7SMauro Carvalho Chehab5) Typedefs 360186128f7SMauro Carvalho Chehab----------- 361186128f7SMauro Carvalho Chehab 362186128f7SMauro Carvalho ChehabPlease don't use things like ``vps_t``. 363186128f7SMauro Carvalho ChehabIt's a **mistake** to use typedef for structures and pointers. When you see a 364186128f7SMauro Carvalho Chehab 365186128f7SMauro Carvalho Chehab.. code-block:: c 366186128f7SMauro Carvalho Chehab 367186128f7SMauro Carvalho Chehab 368186128f7SMauro Carvalho Chehab vps_t a; 369186128f7SMauro Carvalho Chehab 370186128f7SMauro Carvalho Chehabin the source, what does it mean? 371186128f7SMauro Carvalho ChehabIn contrast, if it says 372186128f7SMauro Carvalho Chehab 373186128f7SMauro Carvalho Chehab.. code-block:: c 374186128f7SMauro Carvalho Chehab 375186128f7SMauro Carvalho Chehab struct virtual_container *a; 376186128f7SMauro Carvalho Chehab 377186128f7SMauro Carvalho Chehabyou can actually tell what ``a`` is. 378186128f7SMauro Carvalho Chehab 379186128f7SMauro Carvalho ChehabLots of people think that typedefs ``help readability``. Not so. They are 380186128f7SMauro Carvalho Chehabuseful only for: 381186128f7SMauro Carvalho Chehab 382186128f7SMauro Carvalho Chehab (a) totally opaque objects (where the typedef is actively used to **hide** 383186128f7SMauro Carvalho Chehab what the object is). 384186128f7SMauro Carvalho Chehab 385186128f7SMauro Carvalho Chehab Example: ``pte_t`` etc. opaque objects that you can only access using 386186128f7SMauro Carvalho Chehab the proper accessor functions. 387186128f7SMauro Carvalho Chehab 388186128f7SMauro Carvalho Chehab .. note:: 389186128f7SMauro Carvalho Chehab 390186128f7SMauro Carvalho Chehab Opaqueness and ``accessor functions`` are not good in themselves. 391186128f7SMauro Carvalho Chehab The reason we have them for things like pte_t etc. is that there 392186128f7SMauro Carvalho Chehab really is absolutely **zero** portably accessible information there. 393186128f7SMauro Carvalho Chehab 394186128f7SMauro Carvalho Chehab (b) Clear integer types, where the abstraction **helps** avoid confusion 395186128f7SMauro Carvalho Chehab whether it is ``int`` or ``long``. 396186128f7SMauro Carvalho Chehab 397186128f7SMauro Carvalho Chehab u8/u16/u32 are perfectly fine typedefs, although they fit into 398186128f7SMauro Carvalho Chehab category (d) better than here. 399186128f7SMauro Carvalho Chehab 400186128f7SMauro Carvalho Chehab .. note:: 401186128f7SMauro Carvalho Chehab 402186128f7SMauro Carvalho Chehab Again - there needs to be a **reason** for this. If something is 403186128f7SMauro Carvalho Chehab ``unsigned long``, then there's no reason to do 404186128f7SMauro Carvalho Chehab 405186128f7SMauro Carvalho Chehab typedef unsigned long myflags_t; 406186128f7SMauro Carvalho Chehab 407186128f7SMauro Carvalho Chehab but if there is a clear reason for why it under certain circumstances 408186128f7SMauro Carvalho Chehab might be an ``unsigned int`` and under other configurations might be 409186128f7SMauro Carvalho Chehab ``unsigned long``, then by all means go ahead and use a typedef. 410186128f7SMauro Carvalho Chehab 411186128f7SMauro Carvalho Chehab (c) when you use sparse to literally create a **new** type for 412186128f7SMauro Carvalho Chehab type-checking. 413186128f7SMauro Carvalho Chehab 414186128f7SMauro Carvalho Chehab (d) New types which are identical to standard C99 types, in certain 415186128f7SMauro Carvalho Chehab exceptional circumstances. 416186128f7SMauro Carvalho Chehab 417186128f7SMauro Carvalho Chehab Although it would only take a short amount of time for the eyes and 418186128f7SMauro Carvalho Chehab brain to become accustomed to the standard types like ``uint32_t``, 419186128f7SMauro Carvalho Chehab some people object to their use anyway. 420186128f7SMauro Carvalho Chehab 421186128f7SMauro Carvalho Chehab Therefore, the Linux-specific ``u8/u16/u32/u64`` types and their 422186128f7SMauro Carvalho Chehab signed equivalents which are identical to standard types are 423186128f7SMauro Carvalho Chehab permitted -- although they are not mandatory in new code of your 424186128f7SMauro Carvalho Chehab own. 425186128f7SMauro Carvalho Chehab 426186128f7SMauro Carvalho Chehab When editing existing code which already uses one or the other set 427186128f7SMauro Carvalho Chehab of types, you should conform to the existing choices in that code. 428186128f7SMauro Carvalho Chehab 429186128f7SMauro Carvalho Chehab (e) Types safe for use in userspace. 430186128f7SMauro Carvalho Chehab 431186128f7SMauro Carvalho Chehab In certain structures which are visible to userspace, we cannot 432186128f7SMauro Carvalho Chehab require C99 types and cannot use the ``u32`` form above. Thus, we 433186128f7SMauro Carvalho Chehab use __u32 and similar types in all structures which are shared 434186128f7SMauro Carvalho Chehab with userspace. 435186128f7SMauro Carvalho Chehab 436186128f7SMauro Carvalho ChehabMaybe there are other cases too, but the rule should basically be to NEVER 437186128f7SMauro Carvalho ChehabEVER use a typedef unless you can clearly match one of those rules. 438186128f7SMauro Carvalho Chehab 439186128f7SMauro Carvalho ChehabIn general, a pointer, or a struct that has elements that can reasonably 440186128f7SMauro Carvalho Chehabbe directly accessed should **never** be a typedef. 441186128f7SMauro Carvalho Chehab 442186128f7SMauro Carvalho Chehab 443186128f7SMauro Carvalho Chehab6) Functions 444186128f7SMauro Carvalho Chehab------------ 445186128f7SMauro Carvalho Chehab 446186128f7SMauro Carvalho ChehabFunctions should be short and sweet, and do just one thing. They should 447186128f7SMauro Carvalho Chehabfit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, 448186128f7SMauro Carvalho Chehabas we all know), and do one thing and do that well. 449186128f7SMauro Carvalho Chehab 450186128f7SMauro Carvalho ChehabThe maximum length of a function is inversely proportional to the 451186128f7SMauro Carvalho Chehabcomplexity and indentation level of that function. So, if you have a 452186128f7SMauro Carvalho Chehabconceptually simple function that is just one long (but simple) 453186128f7SMauro Carvalho Chehabcase-statement, where you have to do lots of small things for a lot of 454186128f7SMauro Carvalho Chehabdifferent cases, it's OK to have a longer function. 455186128f7SMauro Carvalho Chehab 456186128f7SMauro Carvalho ChehabHowever, if you have a complex function, and you suspect that a 457186128f7SMauro Carvalho Chehabless-than-gifted first-year high-school student might not even 458186128f7SMauro Carvalho Chehabunderstand what the function is all about, you should adhere to the 459186128f7SMauro Carvalho Chehabmaximum limits all the more closely. Use helper functions with 460186128f7SMauro Carvalho Chehabdescriptive names (you can ask the compiler to in-line them if you think 461186128f7SMauro Carvalho Chehabit's performance-critical, and it will probably do a better job of it 462186128f7SMauro Carvalho Chehabthan you would have done). 463186128f7SMauro Carvalho Chehab 464186128f7SMauro Carvalho ChehabAnother measure of the function is the number of local variables. They 465186128f7SMauro Carvalho Chehabshouldn't exceed 5-10, or you're doing something wrong. Re-think the 466186128f7SMauro Carvalho Chehabfunction, and split it into smaller pieces. A human brain can 467186128f7SMauro Carvalho Chehabgenerally easily keep track of about 7 different things, anything more 468186128f7SMauro Carvalho Chehaband it gets confused. You know you're brilliant, but maybe you'd like 469186128f7SMauro Carvalho Chehabto understand what you did 2 weeks from now. 470186128f7SMauro Carvalho Chehab 471186128f7SMauro Carvalho ChehabIn source files, separate functions with one blank line. If the function is 472186128f7SMauro Carvalho Chehabexported, the **EXPORT** macro for it should follow immediately after the 473186128f7SMauro Carvalho Chehabclosing function brace line. E.g.: 474186128f7SMauro Carvalho Chehab 475186128f7SMauro Carvalho Chehab.. code-block:: c 476186128f7SMauro Carvalho Chehab 477186128f7SMauro Carvalho Chehab int system_is_up(void) 478186128f7SMauro Carvalho Chehab { 479186128f7SMauro Carvalho Chehab return system_state == SYSTEM_RUNNING; 480186128f7SMauro Carvalho Chehab } 481186128f7SMauro Carvalho Chehab EXPORT_SYMBOL(system_is_up); 482186128f7SMauro Carvalho Chehab 483d5b421feSKees Cook6.1) Function prototypes 484d5b421feSKees Cook************************ 485d5b421feSKees Cook 486186128f7SMauro Carvalho ChehabIn function prototypes, include parameter names with their data types. 487186128f7SMauro Carvalho ChehabAlthough this is not required by the C language, it is preferred in Linux 488186128f7SMauro Carvalho Chehabbecause it is a simple way to add valuable information for the reader. 489186128f7SMauro Carvalho Chehab 490d5b421feSKees CookDo not use the ``extern`` keyword with function declarations as this makes 4913fe5dbfeSAlexey Dobriyanlines longer and isn't strictly necessary. 4923fe5dbfeSAlexey Dobriyan 493d5b421feSKees CookWhen writing function prototypes, please keep the `order of elements regular 494d5b421feSKees Cook<https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com/>`_. 495d5b421feSKees CookFor example, using this function declaration example:: 496d5b421feSKees Cook 497d5b421feSKees Cook __init void * __must_check action(enum magic value, size_t size, u8 count, 498d5b421feSKees Cook char *fmt, ...) __printf(4, 5) __malloc; 499d5b421feSKees Cook 500d5b421feSKees CookThe preferred order of elements for a function prototype is: 501d5b421feSKees Cook 502d5b421feSKees Cook- storage class (below, ``static __always_inline``, noting that ``__always_inline`` 503d5b421feSKees Cook is technically an attribute but is treated like ``inline``) 504d5b421feSKees Cook- storage class attributes (here, ``__init`` -- i.e. section declarations, but also 505d5b421feSKees Cook things like ``__cold``) 506d5b421feSKees Cook- return type (here, ``void *``) 507d5b421feSKees Cook- return type attributes (here, ``__must_check``) 508d5b421feSKees Cook- function name (here, ``action``) 509d5b421feSKees Cook- function parameters (here, ``(enum magic value, size_t size, u8 count, char *fmt, ...)``, 510d5b421feSKees Cook noting that parameter names should always be included) 511d5b421feSKees Cook- function parameter attributes (here, ``__printf(4, 5)``) 512d5b421feSKees Cook- function behavior attributes (here, ``__malloc``) 513d5b421feSKees Cook 514d5b421feSKees CookNote that for a function **definition** (i.e. the actual function body), 515d5b421feSKees Cookthe compiler does not allow function parameter attributes after the 516d5b421feSKees Cookfunction parameters. In these cases, they should go after the storage 517d5b421feSKees Cookclass attributes (e.g. note the changed position of ``__printf(4, 5)`` 518d5b421feSKees Cookbelow, compared to the **declaration** example above):: 519d5b421feSKees Cook 520d5b421feSKees Cook static __always_inline __init __printf(4, 5) void * __must_check action(enum magic value, 521d5b421feSKees Cook size_t size, u8 count, char *fmt, ...) __malloc 522d5b421feSKees Cook { 523d5b421feSKees Cook ... 524d5b421feSKees Cook } 525186128f7SMauro Carvalho Chehab 526186128f7SMauro Carvalho Chehab7) Centralized exiting of functions 527186128f7SMauro Carvalho Chehab----------------------------------- 528186128f7SMauro Carvalho Chehab 529186128f7SMauro Carvalho ChehabAlbeit deprecated by some people, the equivalent of the goto statement is 530186128f7SMauro Carvalho Chehabused frequently by compilers in form of the unconditional jump instruction. 531186128f7SMauro Carvalho Chehab 532186128f7SMauro Carvalho ChehabThe goto statement comes in handy when a function exits from multiple 533186128f7SMauro Carvalho Chehablocations and some common work such as cleanup has to be done. If there is no 534186128f7SMauro Carvalho Chehabcleanup needed then just return directly. 535186128f7SMauro Carvalho Chehab 536186128f7SMauro Carvalho ChehabChoose label names which say what the goto does or why the goto exists. An 537186128f7SMauro Carvalho Chehabexample of a good name could be ``out_free_buffer:`` if the goto frees ``buffer``. 538186128f7SMauro Carvalho ChehabAvoid using GW-BASIC names like ``err1:`` and ``err2:``, as you would have to 539186128f7SMauro Carvalho Chehabrenumber them if you ever add or remove exit paths, and they make correctness 540186128f7SMauro Carvalho Chehabdifficult to verify anyway. 541186128f7SMauro Carvalho Chehab 542186128f7SMauro Carvalho ChehabThe rationale for using gotos is: 543186128f7SMauro Carvalho Chehab 544186128f7SMauro Carvalho Chehab- unconditional statements are easier to understand and follow 545186128f7SMauro Carvalho Chehab- nesting is reduced 546186128f7SMauro Carvalho Chehab- errors by not updating individual exit points when making 547186128f7SMauro Carvalho Chehab modifications are prevented 548186128f7SMauro Carvalho Chehab- saves the compiler work to optimize redundant code away ;) 549186128f7SMauro Carvalho Chehab 550186128f7SMauro Carvalho Chehab.. code-block:: c 551186128f7SMauro Carvalho Chehab 552186128f7SMauro Carvalho Chehab int fun(int a) 553186128f7SMauro Carvalho Chehab { 554186128f7SMauro Carvalho Chehab int result = 0; 555186128f7SMauro Carvalho Chehab char *buffer; 556186128f7SMauro Carvalho Chehab 557186128f7SMauro Carvalho Chehab buffer = kmalloc(SIZE, GFP_KERNEL); 558186128f7SMauro Carvalho Chehab if (!buffer) 559186128f7SMauro Carvalho Chehab return -ENOMEM; 560186128f7SMauro Carvalho Chehab 561186128f7SMauro Carvalho Chehab if (condition1) { 562186128f7SMauro Carvalho Chehab while (loop1) { 563186128f7SMauro Carvalho Chehab ... 564186128f7SMauro Carvalho Chehab } 565186128f7SMauro Carvalho Chehab result = 1; 566beab6cb2SMasahiro Yamada goto out_free_buffer; 567186128f7SMauro Carvalho Chehab } 568186128f7SMauro Carvalho Chehab ... 569186128f7SMauro Carvalho Chehab out_free_buffer: 570186128f7SMauro Carvalho Chehab kfree(buffer); 571186128f7SMauro Carvalho Chehab return result; 572186128f7SMauro Carvalho Chehab } 573186128f7SMauro Carvalho Chehab 574186128f7SMauro Carvalho ChehabA common type of bug to be aware of is ``one err bugs`` which look like this: 575186128f7SMauro Carvalho Chehab 576186128f7SMauro Carvalho Chehab.. code-block:: c 577186128f7SMauro Carvalho Chehab 578186128f7SMauro Carvalho Chehab err: 579186128f7SMauro Carvalho Chehab kfree(foo->bar); 580186128f7SMauro Carvalho Chehab kfree(foo); 581186128f7SMauro Carvalho Chehab return ret; 582186128f7SMauro Carvalho Chehab 583186128f7SMauro Carvalho ChehabThe bug in this code is that on some exit paths ``foo`` is NULL. Normally the 584186128f7SMauro Carvalho Chehabfix for this is to split it up into two error labels ``err_free_bar:`` and 585186128f7SMauro Carvalho Chehab``err_free_foo:``: 586186128f7SMauro Carvalho Chehab 587186128f7SMauro Carvalho Chehab.. code-block:: c 588186128f7SMauro Carvalho Chehab 589186128f7SMauro Carvalho Chehab err_free_bar: 590186128f7SMauro Carvalho Chehab kfree(foo->bar); 591186128f7SMauro Carvalho Chehab err_free_foo: 592186128f7SMauro Carvalho Chehab kfree(foo); 593186128f7SMauro Carvalho Chehab return ret; 594186128f7SMauro Carvalho Chehab 595186128f7SMauro Carvalho ChehabIdeally you should simulate errors to test all exit paths. 596186128f7SMauro Carvalho Chehab 597186128f7SMauro Carvalho Chehab 598186128f7SMauro Carvalho Chehab8) Commenting 599186128f7SMauro Carvalho Chehab------------- 600186128f7SMauro Carvalho Chehab 601186128f7SMauro Carvalho ChehabComments are good, but there is also a danger of over-commenting. NEVER 602186128f7SMauro Carvalho Chehabtry to explain HOW your code works in a comment: it's much better to 603186128f7SMauro Carvalho Chehabwrite the code so that the **working** is obvious, and it's a waste of 604186128f7SMauro Carvalho Chehabtime to explain badly written code. 605186128f7SMauro Carvalho Chehab 606186128f7SMauro Carvalho ChehabGenerally, you want your comments to tell WHAT your code does, not HOW. 607186128f7SMauro Carvalho ChehabAlso, try to avoid putting comments inside a function body: if the 608186128f7SMauro Carvalho Chehabfunction is so complex that you need to separately comment parts of it, 609186128f7SMauro Carvalho Chehabyou should probably go back to chapter 6 for a while. You can make 610186128f7SMauro Carvalho Chehabsmall comments to note or warn about something particularly clever (or 611186128f7SMauro Carvalho Chehabugly), but try to avoid excess. Instead, put the comments at the head 612186128f7SMauro Carvalho Chehabof the function, telling people what it does, and possibly WHY it does 613186128f7SMauro Carvalho Chehabit. 614186128f7SMauro Carvalho Chehab 615186128f7SMauro Carvalho ChehabWhen commenting the kernel API functions, please use the kernel-doc format. 6161dc4bbf0SMauro Carvalho ChehabSee the files at :ref:`Documentation/doc-guide/ <doc_guide>` and 6171dc4bbf0SMauro Carvalho Chehab``scripts/kernel-doc`` for details. 618186128f7SMauro Carvalho Chehab 619186128f7SMauro Carvalho ChehabThe preferred style for long (multi-line) comments is: 620186128f7SMauro Carvalho Chehab 621186128f7SMauro Carvalho Chehab.. code-block:: c 622186128f7SMauro Carvalho Chehab 623186128f7SMauro Carvalho Chehab /* 624186128f7SMauro Carvalho Chehab * This is the preferred style for multi-line 625186128f7SMauro Carvalho Chehab * comments in the Linux kernel source code. 626186128f7SMauro Carvalho Chehab * Please use it consistently. 627186128f7SMauro Carvalho Chehab * 628186128f7SMauro Carvalho Chehab * Description: A column of asterisks on the left side, 629186128f7SMauro Carvalho Chehab * with beginning and ending almost-blank lines. 630186128f7SMauro Carvalho Chehab */ 631186128f7SMauro Carvalho Chehab 632186128f7SMauro Carvalho ChehabIt's also important to comment data, whether they are basic types or derived 633186128f7SMauro Carvalho Chehabtypes. To this end, use just one data declaration per line (no commas for 634186128f7SMauro Carvalho Chehabmultiple data declarations). This leaves you room for a small comment on each 635186128f7SMauro Carvalho Chehabitem, explaining its use. 636186128f7SMauro Carvalho Chehab 637186128f7SMauro Carvalho Chehab 638186128f7SMauro Carvalho Chehab9) You've made a mess of it 639186128f7SMauro Carvalho Chehab--------------------------- 640186128f7SMauro Carvalho Chehab 641186128f7SMauro Carvalho ChehabThat's OK, we all do. You've probably been told by your long-time Unix 642186128f7SMauro Carvalho Chehabuser helper that ``GNU emacs`` automatically formats the C sources for 643186128f7SMauro Carvalho Chehabyou, and you've noticed that yes, it does do that, but the defaults it 644186128f7SMauro Carvalho Chehabuses are less than desirable (in fact, they are worse than random 645186128f7SMauro Carvalho Chehabtyping - an infinite number of monkeys typing into GNU emacs would never 646186128f7SMauro Carvalho Chehabmake a good program). 647186128f7SMauro Carvalho Chehab 648186128f7SMauro Carvalho ChehabSo, you can either get rid of GNU emacs, or change it to use saner 649186128f7SMauro Carvalho Chehabvalues. To do the latter, you can stick the following in your .emacs file: 650186128f7SMauro Carvalho Chehab 651185ea767SThorsten Blum.. code-block:: elisp 652186128f7SMauro Carvalho Chehab 653186128f7SMauro Carvalho Chehab (defun c-lineup-arglist-tabs-only (ignored) 654186128f7SMauro Carvalho Chehab "Line up argument lists by tabs, not spaces" 655186128f7SMauro Carvalho Chehab (let* ((anchor (c-langelem-pos c-syntactic-element)) 656186128f7SMauro Carvalho Chehab (column (c-langelem-2nd-pos c-syntactic-element)) 657186128f7SMauro Carvalho Chehab (offset (- (1+ column) anchor)) 658186128f7SMauro Carvalho Chehab (steps (floor offset c-basic-offset))) 659186128f7SMauro Carvalho Chehab (* (max steps 1) 660186128f7SMauro Carvalho Chehab c-basic-offset))) 661186128f7SMauro Carvalho Chehab 6622fec7b33SBart Van Assche (dir-locals-set-class-variables 6632fec7b33SBart Van Assche 'linux-kernel 6642fec7b33SBart Van Assche '((c-mode . ( 6652fec7b33SBart Van Assche (c-basic-offset . 8) 6662fec7b33SBart Van Assche (c-label-minimum-indentation . 0) 6672fec7b33SBart Van Assche (c-offsets-alist . ( 6682fec7b33SBart Van Assche (arglist-close . c-lineup-arglist-tabs-only) 6692fec7b33SBart Van Assche (arglist-cont-nonempty . 6702fec7b33SBart Van Assche (c-lineup-gcc-asm-reg c-lineup-arglist-tabs-only)) 6712fec7b33SBart Van Assche (arglist-intro . +) 6722fec7b33SBart Van Assche (brace-list-intro . +) 6732fec7b33SBart Van Assche (c . c-lineup-C-comments) 6742fec7b33SBart Van Assche (case-label . 0) 6752fec7b33SBart Van Assche (comment-intro . c-lineup-comment) 6762fec7b33SBart Van Assche (cpp-define-intro . +) 6772fec7b33SBart Van Assche (cpp-macro . -1000) 6782fec7b33SBart Van Assche (cpp-macro-cont . +) 6792fec7b33SBart Van Assche (defun-block-intro . +) 6802fec7b33SBart Van Assche (else-clause . 0) 6812fec7b33SBart Van Assche (func-decl-cont . +) 6822fec7b33SBart Van Assche (inclass . +) 6832fec7b33SBart Van Assche (inher-cont . c-lineup-multi-inher) 6842fec7b33SBart Van Assche (knr-argdecl-intro . 0) 6852fec7b33SBart Van Assche (label . -1000) 6862fec7b33SBart Van Assche (statement . 0) 6872fec7b33SBart Van Assche (statement-block-intro . +) 6882fec7b33SBart Van Assche (statement-case-intro . +) 6892fec7b33SBart Van Assche (statement-cont . +) 6902fec7b33SBart Van Assche (substatement . +) 6912fec7b33SBart Van Assche )) 6922fec7b33SBart Van Assche (indent-tabs-mode . t) 6932fec7b33SBart Van Assche (show-trailing-whitespace . t) 6942fec7b33SBart Van Assche )))) 695186128f7SMauro Carvalho Chehab 6962fec7b33SBart Van Assche (dir-locals-set-directory-class 6972fec7b33SBart Van Assche (expand-file-name "~/src/linux-trees") 6982fec7b33SBart Van Assche 'linux-kernel) 699186128f7SMauro Carvalho Chehab 700186128f7SMauro Carvalho ChehabThis will make emacs go better with the kernel coding style for C 701186128f7SMauro Carvalho Chehabfiles below ``~/src/linux-trees``. 702186128f7SMauro Carvalho Chehab 703186128f7SMauro Carvalho ChehabBut even if you fail in getting emacs to do sane formatting, not 704186128f7SMauro Carvalho Chehabeverything is lost: use ``indent``. 705186128f7SMauro Carvalho Chehab 706186128f7SMauro Carvalho ChehabNow, again, GNU indent has the same brain-dead settings that GNU emacs 707186128f7SMauro Carvalho Chehabhas, which is why you need to give it a few command line options. 708186128f7SMauro Carvalho ChehabHowever, that's not too bad, because even the makers of GNU indent 709186128f7SMauro Carvalho Chehabrecognize the authority of K&R (the GNU people aren't evil, they are 710186128f7SMauro Carvalho Chehabjust severely misguided in this matter), so you just give indent the 711186128f7SMauro Carvalho Chehaboptions ``-kr -i8`` (stands for ``K&R, 8 character indents``), or use 712186128f7SMauro Carvalho Chehab``scripts/Lindent``, which indents in the latest style. 713186128f7SMauro Carvalho Chehab 714186128f7SMauro Carvalho Chehab``indent`` has a lot of options, and especially when it comes to comment 715186128f7SMauro Carvalho Chehabre-formatting you may want to take a look at the man page. But 716186128f7SMauro Carvalho Chehabremember: ``indent`` is not a fix for bad programming. 717186128f7SMauro Carvalho Chehab 718d4ef8d3fSMiguel OjedaNote that you can also use the ``clang-format`` tool to help you with 719d4ef8d3fSMiguel Ojedathese rules, to quickly re-format parts of your code automatically, 720d4ef8d3fSMiguel Ojedaand to review full files in order to spot coding style mistakes, 721d4ef8d3fSMiguel Ojedatypos and possible improvements. It is also handy for sorting ``#includes``, 722d4ef8d3fSMiguel Ojedafor aligning variables/macros, for reflowing text and other similar tasks. 723*e3b10a02SSeongJae ParkSee the file :ref:`Documentation/dev-tools/clang-format.rst <clangformat>` 724d4ef8d3fSMiguel Ojedafor more details. 725d4ef8d3fSMiguel Ojeda 7265a602de9SÍñigo HuguetSome basic editor settings, such as indentation and line endings, will be 7275a602de9SÍñigo Huguetset automatically if you are using an editor that is compatible with 7285a602de9SÍñigo HuguetEditorConfig. See the official EditorConfig website for more information: 7295a602de9SÍñigo Huguethttps://editorconfig.org/ 730186128f7SMauro Carvalho Chehab 731186128f7SMauro Carvalho Chehab10) Kconfig configuration files 732186128f7SMauro Carvalho Chehab------------------------------- 733186128f7SMauro Carvalho Chehab 734186128f7SMauro Carvalho ChehabFor all of the Kconfig* configuration files throughout the source tree, 735186128f7SMauro Carvalho Chehabthe indentation is somewhat different. Lines under a ``config`` definition 736186128f7SMauro Carvalho Chehabare indented with one tab, while help text is indented an additional two 737186128f7SMauro Carvalho Chehabspaces. Example:: 738186128f7SMauro Carvalho Chehab 739186128f7SMauro Carvalho Chehab config AUDIT 740186128f7SMauro Carvalho Chehab bool "Auditing support" 741186128f7SMauro Carvalho Chehab depends on NET 742186128f7SMauro Carvalho Chehab help 743186128f7SMauro Carvalho Chehab Enable auditing infrastructure that can be used with another 744186128f7SMauro Carvalho Chehab kernel subsystem, such as SELinux (which requires this for 745186128f7SMauro Carvalho Chehab logging of avc messages output). Does not do system-call 746186128f7SMauro Carvalho Chehab auditing without CONFIG_AUDITSYSCALL. 747186128f7SMauro Carvalho Chehab 748186128f7SMauro Carvalho ChehabSeriously dangerous features (such as write support for certain 749186128f7SMauro Carvalho Chehabfilesystems) should advertise this prominently in their prompt string:: 750186128f7SMauro Carvalho Chehab 751186128f7SMauro Carvalho Chehab config ADFS_FS_RW 752186128f7SMauro Carvalho Chehab bool "ADFS write support (DANGEROUS)" 753186128f7SMauro Carvalho Chehab depends on ADFS_FS 754186128f7SMauro Carvalho Chehab ... 755186128f7SMauro Carvalho Chehab 756186128f7SMauro Carvalho ChehabFor full documentation on the configuration files, see the file 757cd238effSMauro Carvalho ChehabDocumentation/kbuild/kconfig-language.rst. 758186128f7SMauro Carvalho Chehab 759186128f7SMauro Carvalho Chehab 760186128f7SMauro Carvalho Chehab11) Data structures 761186128f7SMauro Carvalho Chehab------------------- 762186128f7SMauro Carvalho Chehab 763186128f7SMauro Carvalho ChehabData structures that have visibility outside the single-threaded 764186128f7SMauro Carvalho Chehabenvironment they are created and destroyed in should always have 765186128f7SMauro Carvalho Chehabreference counts. In the kernel, garbage collection doesn't exist (and 766186128f7SMauro Carvalho Chehaboutside the kernel garbage collection is slow and inefficient), which 767186128f7SMauro Carvalho Chehabmeans that you absolutely **have** to reference count all your uses. 768186128f7SMauro Carvalho Chehab 769186128f7SMauro Carvalho ChehabReference counting means that you can avoid locking, and allows multiple 770186128f7SMauro Carvalho Chehabusers to have access to the data structure in parallel - and not having 771186128f7SMauro Carvalho Chehabto worry about the structure suddenly going away from under them just 772186128f7SMauro Carvalho Chehabbecause they slept or did something else for a while. 773186128f7SMauro Carvalho Chehab 774186128f7SMauro Carvalho ChehabNote that locking is **not** a replacement for reference counting. 775186128f7SMauro Carvalho ChehabLocking is used to keep data structures coherent, while reference 776186128f7SMauro Carvalho Chehabcounting is a memory management technique. Usually both are needed, and 777186128f7SMauro Carvalho Chehabthey are not to be confused with each other. 778186128f7SMauro Carvalho Chehab 779186128f7SMauro Carvalho ChehabMany data structures can indeed have two levels of reference counting, 780186128f7SMauro Carvalho Chehabwhen there are users of different ``classes``. The subclass count counts 781186128f7SMauro Carvalho Chehabthe number of subclass users, and decrements the global count just once 782186128f7SMauro Carvalho Chehabwhen the subclass count goes to zero. 783186128f7SMauro Carvalho Chehab 784186128f7SMauro Carvalho ChehabExamples of this kind of ``multi-level-reference-counting`` can be found in 785186128f7SMauro Carvalho Chehabmemory management (``struct mm_struct``: mm_users and mm_count), and in 786186128f7SMauro Carvalho Chehabfilesystem code (``struct super_block``: s_count and s_active). 787186128f7SMauro Carvalho Chehab 788186128f7SMauro Carvalho ChehabRemember: if another thread can find your data structure, and you don't 789186128f7SMauro Carvalho Chehabhave a reference count on it, you almost certainly have a bug. 790186128f7SMauro Carvalho Chehab 791186128f7SMauro Carvalho Chehab 792186128f7SMauro Carvalho Chehab12) Macros, Enums and RTL 793186128f7SMauro Carvalho Chehab------------------------- 794186128f7SMauro Carvalho Chehab 795186128f7SMauro Carvalho ChehabNames of macros defining constants and labels in enums are capitalized. 796186128f7SMauro Carvalho Chehab 797186128f7SMauro Carvalho Chehab.. code-block:: c 798186128f7SMauro Carvalho Chehab 799186128f7SMauro Carvalho Chehab #define CONSTANT 0x12345 800186128f7SMauro Carvalho Chehab 801186128f7SMauro Carvalho ChehabEnums are preferred when defining several related constants. 802186128f7SMauro Carvalho Chehab 803186128f7SMauro Carvalho ChehabCAPITALIZED macro names are appreciated but macros resembling functions 804186128f7SMauro Carvalho Chehabmay be named in lower case. 805186128f7SMauro Carvalho Chehab 806186128f7SMauro Carvalho ChehabGenerally, inline functions are preferable to macros resembling functions. 807186128f7SMauro Carvalho Chehab 808186128f7SMauro Carvalho ChehabMacros with multiple statements should be enclosed in a do - while block: 809186128f7SMauro Carvalho Chehab 810186128f7SMauro Carvalho Chehab.. code-block:: c 811186128f7SMauro Carvalho Chehab 812186128f7SMauro Carvalho Chehab #define macrofun(a, b, c) \ 813186128f7SMauro Carvalho Chehab do { \ 814186128f7SMauro Carvalho Chehab if (a == 5) \ 815186128f7SMauro Carvalho Chehab do_this(b, c); \ 816186128f7SMauro Carvalho Chehab } while (0) 817186128f7SMauro Carvalho Chehab 8186813216bSBarry SongFunction-like macros with unused parameters should be replaced by static 8196813216bSBarry Songinline functions to avoid the issue of unused variables: 8206813216bSBarry Song 8216813216bSBarry Song.. code-block:: c 8226813216bSBarry Song 8236813216bSBarry Song static inline void fun(struct foo *foo) 8246813216bSBarry Song { 8256813216bSBarry Song } 8266813216bSBarry Song 8276813216bSBarry SongDue to historical practices, many files still employ the "cast to (void)" 8286813216bSBarry Songapproach to evaluate parameters. However, this method is not advisable. 8296813216bSBarry SongInline functions address the issue of "expression with side effects 8306813216bSBarry Songevaluated more than once", circumvent unused-variable problems, and 8316813216bSBarry Songare generally better documented than macros for some reason. 8326813216bSBarry Song 8336813216bSBarry Song.. code-block:: c 8346813216bSBarry Song 8356813216bSBarry Song /* 8366813216bSBarry Song * Avoid doing this whenever possible and instead opt for static 8376813216bSBarry Song * inline functions 8386813216bSBarry Song */ 8396813216bSBarry Song #define macrofun(foo) do { (void) (foo); } while (0) 8406813216bSBarry Song 841186128f7SMauro Carvalho ChehabThings to avoid when using macros: 842186128f7SMauro Carvalho Chehab 843186128f7SMauro Carvalho Chehab1) macros that affect control flow: 844186128f7SMauro Carvalho Chehab 845186128f7SMauro Carvalho Chehab.. code-block:: c 846186128f7SMauro Carvalho Chehab 847186128f7SMauro Carvalho Chehab #define FOO(x) \ 848186128f7SMauro Carvalho Chehab do { \ 849186128f7SMauro Carvalho Chehab if (blah(x) < 0) \ 850186128f7SMauro Carvalho Chehab return -EBUGGERED; \ 851186128f7SMauro Carvalho Chehab } while (0) 852186128f7SMauro Carvalho Chehab 853186128f7SMauro Carvalho Chehabis a **very** bad idea. It looks like a function call but exits the ``calling`` 854186128f7SMauro Carvalho Chehabfunction; don't break the internal parsers of those who will read the code. 855186128f7SMauro Carvalho Chehab 856186128f7SMauro Carvalho Chehab2) macros that depend on having a local variable with a magic name: 857186128f7SMauro Carvalho Chehab 858186128f7SMauro Carvalho Chehab.. code-block:: c 859186128f7SMauro Carvalho Chehab 860186128f7SMauro Carvalho Chehab #define FOO(val) bar(index, val) 861186128f7SMauro Carvalho Chehab 862186128f7SMauro Carvalho Chehabmight look like a good thing, but it's confusing as hell when one reads the 863186128f7SMauro Carvalho Chehabcode and it's prone to breakage from seemingly innocent changes. 864186128f7SMauro Carvalho Chehab 865186128f7SMauro Carvalho Chehab3) macros with arguments that are used as l-values: FOO(x) = y; will 866186128f7SMauro Carvalho Chehabbite you if somebody e.g. turns FOO into an inline function. 867186128f7SMauro Carvalho Chehab 868186128f7SMauro Carvalho Chehab4) forgetting about precedence: macros defining constants using expressions 869186128f7SMauro Carvalho Chehabmust enclose the expression in parentheses. Beware of similar issues with 870186128f7SMauro Carvalho Chehabmacros using parameters. 871186128f7SMauro Carvalho Chehab 872186128f7SMauro Carvalho Chehab.. code-block:: c 873186128f7SMauro Carvalho Chehab 874186128f7SMauro Carvalho Chehab #define CONSTANT 0x4000 875186128f7SMauro Carvalho Chehab #define CONSTEXP (CONSTANT | 3) 876186128f7SMauro Carvalho Chehab 877186128f7SMauro Carvalho Chehab5) namespace collisions when defining local variables in macros resembling 878186128f7SMauro Carvalho Chehabfunctions: 879186128f7SMauro Carvalho Chehab 880186128f7SMauro Carvalho Chehab.. code-block:: c 881186128f7SMauro Carvalho Chehab 882186128f7SMauro Carvalho Chehab #define FOO(x) \ 883186128f7SMauro Carvalho Chehab ({ \ 884186128f7SMauro Carvalho Chehab typeof(x) ret; \ 885186128f7SMauro Carvalho Chehab ret = calc_ret(x); \ 886186128f7SMauro Carvalho Chehab (ret); \ 887186128f7SMauro Carvalho Chehab }) 888186128f7SMauro Carvalho Chehab 889186128f7SMauro Carvalho Chehabret is a common name for a local variable - __foo_ret is less likely 890186128f7SMauro Carvalho Chehabto collide with an existing variable. 891186128f7SMauro Carvalho Chehab 892186128f7SMauro Carvalho ChehabThe cpp manual deals with macros exhaustively. The gcc internals manual also 893186128f7SMauro Carvalho Chehabcovers RTL which is used frequently with assembly language in the kernel. 894186128f7SMauro Carvalho Chehab 895186128f7SMauro Carvalho Chehab 896186128f7SMauro Carvalho Chehab13) Printing kernel messages 897186128f7SMauro Carvalho Chehab---------------------------- 898186128f7SMauro Carvalho Chehab 899186128f7SMauro Carvalho ChehabKernel developers like to be seen as literate. Do mind the spelling 900fb0e0ffeSTony Fischettiof kernel messages to make a good impression. Do not use incorrect 901fb0e0ffeSTony Fischetticontractions like ``dont``; use ``do not`` or ``don't`` instead. Make the 902fb0e0ffeSTony Fischettimessages concise, clear, and unambiguous. 903186128f7SMauro Carvalho Chehab 904186128f7SMauro Carvalho ChehabKernel messages do not have to be terminated with a period. 905186128f7SMauro Carvalho Chehab 906186128f7SMauro Carvalho ChehabPrinting numbers in parentheses (%d) adds no value and should be avoided. 907186128f7SMauro Carvalho Chehab 908c04639a7STrevor WoernerThere are a number of driver model diagnostic macros in <linux/dev_printk.h> 909186128f7SMauro Carvalho Chehabwhich you should use to make sure messages are matched to the right device 910186128f7SMauro Carvalho Chehaband driver, and are tagged with the right level: dev_err(), dev_warn(), 911186128f7SMauro Carvalho Chehabdev_info(), and so forth. For messages that aren't associated with a 912186128f7SMauro Carvalho Chehabparticular device, <linux/printk.h> defines pr_notice(), pr_info(), 9135c7944caSAbhishek Pandit-Subedipr_warn(), pr_err(), etc. When drivers are working properly they are quiet, 9145c7944caSAbhishek Pandit-Subediso prefer to use dev_dbg/pr_debug unless something is wrong. 915186128f7SMauro Carvalho Chehab 916186128f7SMauro Carvalho ChehabComing up with good debugging messages can be quite a challenge; and once 917186128f7SMauro Carvalho Chehabyou have them, they can be a huge help for remote troubleshooting. However 918186128f7SMauro Carvalho Chehabdebug message printing is handled differently than printing other non-debug 919186128f7SMauro Carvalho Chehabmessages. While the other pr_XXX() functions print unconditionally, 920186128f7SMauro Carvalho Chehabpr_debug() does not; it is compiled out by default, unless either DEBUG is 921186128f7SMauro Carvalho Chehabdefined or CONFIG_DYNAMIC_DEBUG is set. That is true for dev_dbg() also, 922186128f7SMauro Carvalho Chehaband a related convention uses VERBOSE_DEBUG to add dev_vdbg() messages to 923186128f7SMauro Carvalho Chehabthe ones already enabled by DEBUG. 924186128f7SMauro Carvalho Chehab 925186128f7SMauro Carvalho ChehabMany subsystems have Kconfig debug options to turn on -DDEBUG in the 926186128f7SMauro Carvalho Chehabcorresponding Makefile; in other cases specific files #define DEBUG. And 927186128f7SMauro Carvalho Chehabwhen a debug message should be unconditionally printed, such as if it is 928186128f7SMauro Carvalho Chehabalready inside a debug-related #ifdef section, printk(KERN_DEBUG ...) can be 929186128f7SMauro Carvalho Chehabused. 930186128f7SMauro Carvalho Chehab 931186128f7SMauro Carvalho Chehab 932186128f7SMauro Carvalho Chehab14) Allocating memory 933186128f7SMauro Carvalho Chehab--------------------- 934186128f7SMauro Carvalho Chehab 935186128f7SMauro Carvalho ChehabThe kernel provides the following general purpose memory allocators: 936186128f7SMauro Carvalho Chehabkmalloc(), kzalloc(), kmalloc_array(), kcalloc(), vmalloc(), and 937186128f7SMauro Carvalho Chehabvzalloc(). Please refer to the API documentation for further information 938bba757d8SJoe Perchesabout them. :ref:`Documentation/core-api/memory-allocation.rst 939bba757d8SJoe Perches<memory_allocation>` 940186128f7SMauro Carvalho Chehab 941186128f7SMauro Carvalho ChehabThe preferred form for passing a size of a struct is the following: 942186128f7SMauro Carvalho Chehab 943186128f7SMauro Carvalho Chehab.. code-block:: c 944186128f7SMauro Carvalho Chehab 945186128f7SMauro Carvalho Chehab p = kmalloc(sizeof(*p), ...); 946186128f7SMauro Carvalho Chehab 947186128f7SMauro Carvalho ChehabThe alternative form where struct name is spelled out hurts readability and 948186128f7SMauro Carvalho Chehabintroduces an opportunity for a bug when the pointer variable type is changed 949186128f7SMauro Carvalho Chehabbut the corresponding sizeof that is passed to a memory allocator is not. 950186128f7SMauro Carvalho Chehab 951186128f7SMauro Carvalho ChehabCasting the return value which is a void pointer is redundant. The conversion 952186128f7SMauro Carvalho Chehabfrom void pointer to any other pointer type is guaranteed by the C programming 953186128f7SMauro Carvalho Chehablanguage. 954186128f7SMauro Carvalho Chehab 955186128f7SMauro Carvalho ChehabThe preferred form for allocating an array is the following: 956186128f7SMauro Carvalho Chehab 957186128f7SMauro Carvalho Chehab.. code-block:: c 958186128f7SMauro Carvalho Chehab 959186128f7SMauro Carvalho Chehab p = kmalloc_array(n, sizeof(...), ...); 960186128f7SMauro Carvalho Chehab 961186128f7SMauro Carvalho ChehabThe preferred form for allocating a zeroed array is the following: 962186128f7SMauro Carvalho Chehab 963186128f7SMauro Carvalho Chehab.. code-block:: c 964186128f7SMauro Carvalho Chehab 965186128f7SMauro Carvalho Chehab p = kcalloc(n, sizeof(...), ...); 966186128f7SMauro Carvalho Chehab 967186128f7SMauro Carvalho ChehabBoth forms check for overflow on the allocation size n * sizeof(...), 968186128f7SMauro Carvalho Chehaband return NULL if that occurred. 969186128f7SMauro Carvalho Chehab 970bba757d8SJoe PerchesThese generic allocation functions all emit a stack dump on failure when used 971bba757d8SJoe Percheswithout __GFP_NOWARN so there is no use in emitting an additional failure 972bba757d8SJoe Perchesmessage when NULL is returned. 973186128f7SMauro Carvalho Chehab 974186128f7SMauro Carvalho Chehab15) The inline disease 975186128f7SMauro Carvalho Chehab---------------------- 976186128f7SMauro Carvalho Chehab 977186128f7SMauro Carvalho ChehabThere appears to be a common misperception that gcc has a magic "make me 978186128f7SMauro Carvalho Chehabfaster" speedup option called ``inline``. While the use of inlines can be 979186128f7SMauro Carvalho Chehabappropriate (for example as a means of replacing macros, see Chapter 12), it 980186128f7SMauro Carvalho Chehabvery often is not. Abundant use of the inline keyword leads to a much bigger 981186128f7SMauro Carvalho Chehabkernel, which in turn slows the system as a whole down, due to a bigger 982186128f7SMauro Carvalho Chehabicache footprint for the CPU and simply because there is less memory 983186128f7SMauro Carvalho Chehabavailable for the pagecache. Just think about it; a pagecache miss causes a 984186128f7SMauro Carvalho Chehabdisk seek, which easily takes 5 milliseconds. There are a LOT of cpu cycles 985186128f7SMauro Carvalho Chehabthat can go into these 5 milliseconds. 986186128f7SMauro Carvalho Chehab 987186128f7SMauro Carvalho ChehabA reasonable rule of thumb is to not put inline at functions that have more 988186128f7SMauro Carvalho Chehabthan 3 lines of code in them. An exception to this rule are the cases where 989186128f7SMauro Carvalho Chehaba parameter is known to be a compile time constant, and as a result of this 990186128f7SMauro Carvalho Chehabconstantness you *know* the compiler will be able to optimize most of your 991186128f7SMauro Carvalho Chehabfunction away at compile time. For a good example of this later case, see 992186128f7SMauro Carvalho Chehabthe kmalloc() inline function. 993186128f7SMauro Carvalho Chehab 994186128f7SMauro Carvalho ChehabOften people argue that adding inline to functions that are static and used 995186128f7SMauro Carvalho Chehabonly once is always a win since there is no space tradeoff. While this is 996186128f7SMauro Carvalho Chehabtechnically correct, gcc is capable of inlining these automatically without 997186128f7SMauro Carvalho Chehabhelp, and the maintenance issue of removing the inline when a second user 998186128f7SMauro Carvalho Chehabappears outweighs the potential value of the hint that tells gcc to do 999186128f7SMauro Carvalho Chehabsomething it would have done anyway. 1000186128f7SMauro Carvalho Chehab 1001186128f7SMauro Carvalho Chehab 1002186128f7SMauro Carvalho Chehab16) Function return values and names 1003186128f7SMauro Carvalho Chehab------------------------------------ 1004186128f7SMauro Carvalho Chehab 1005186128f7SMauro Carvalho ChehabFunctions can return values of many different kinds, and one of the 1006186128f7SMauro Carvalho Chehabmost common is a value indicating whether the function succeeded or 1007186128f7SMauro Carvalho Chehabfailed. Such a value can be represented as an error-code integer 1008186128f7SMauro Carvalho Chehab(-Exxx = failure, 0 = success) or a ``succeeded`` boolean (0 = failure, 1009186128f7SMauro Carvalho Chehabnon-zero = success). 1010186128f7SMauro Carvalho Chehab 1011186128f7SMauro Carvalho ChehabMixing up these two sorts of representations is a fertile source of 1012186128f7SMauro Carvalho Chehabdifficult-to-find bugs. If the C language included a strong distinction 1013186128f7SMauro Carvalho Chehabbetween integers and booleans then the compiler would find these mistakes 1014186128f7SMauro Carvalho Chehabfor us... but it doesn't. To help prevent such bugs, always follow this 1015186128f7SMauro Carvalho Chehabconvention:: 1016186128f7SMauro Carvalho Chehab 1017186128f7SMauro Carvalho Chehab If the name of a function is an action or an imperative command, 1018186128f7SMauro Carvalho Chehab the function should return an error-code integer. If the name 1019186128f7SMauro Carvalho Chehab is a predicate, the function should return a "succeeded" boolean. 1020186128f7SMauro Carvalho Chehab 1021186128f7SMauro Carvalho ChehabFor example, ``add work`` is a command, and the add_work() function returns 0 1022186128f7SMauro Carvalho Chehabfor success or -EBUSY for failure. In the same way, ``PCI device present`` is 1023186128f7SMauro Carvalho Chehaba predicate, and the pci_dev_present() function returns 1 if it succeeds in 1024186128f7SMauro Carvalho Chehabfinding a matching device or 0 if it doesn't. 1025186128f7SMauro Carvalho Chehab 1026186128f7SMauro Carvalho ChehabAll EXPORTed functions must respect this convention, and so should all 1027186128f7SMauro Carvalho Chehabpublic functions. Private (static) functions need not, but it is 1028186128f7SMauro Carvalho Chehabrecommended that they do. 1029186128f7SMauro Carvalho Chehab 1030186128f7SMauro Carvalho ChehabFunctions whose return value is the actual result of a computation, rather 1031186128f7SMauro Carvalho Chehabthan an indication of whether the computation succeeded, are not subject to 1032186128f7SMauro Carvalho Chehabthis rule. Generally they indicate failure by returning some out-of-range 1033186128f7SMauro Carvalho Chehabresult. Typical examples would be functions that return pointers; they use 1034186128f7SMauro Carvalho ChehabNULL or the ERR_PTR mechanism to report failure. 1035186128f7SMauro Carvalho Chehab 1036186128f7SMauro Carvalho Chehab 10377967656fSJason Gunthorpe17) Using bool 10387967656fSJason Gunthorpe-------------- 10397967656fSJason Gunthorpe 10407967656fSJason GunthorpeThe Linux kernel bool type is an alias for the C99 _Bool type. bool values can 10417967656fSJason Gunthorpeonly evaluate to 0 or 1, and implicit or explicit conversion to bool 10427967656fSJason Gunthorpeautomatically converts the value to true or false. When using bool types the 10437967656fSJason Gunthorpe!! construction is not needed, which eliminates a class of bugs. 10447967656fSJason Gunthorpe 10457967656fSJason GunthorpeWhen working with bool values the true and false definitions should be used 10467967656fSJason Gunthorpeinstead of 1 and 0. 10477967656fSJason Gunthorpe 10487967656fSJason Gunthorpebool function return types and stack variables are always fine to use whenever 10497967656fSJason Gunthorpeappropriate. Use of bool is encouraged to improve readability and is often a 10507967656fSJason Gunthorpebetter option than 'int' for storing boolean values. 10517967656fSJason Gunthorpe 10527967656fSJason GunthorpeDo not use bool if cache line layout or size of the value matters, as its size 10537967656fSJason Gunthorpeand alignment varies based on the compiled architecture. Structures that are 10547967656fSJason Gunthorpeoptimized for alignment and size should not use bool. 10557967656fSJason Gunthorpe 10567967656fSJason GunthorpeIf a structure has many true/false values, consider consolidating them into a 10577967656fSJason Gunthorpebitfield with 1 bit members, or using an appropriate fixed width type, such as 10587967656fSJason Gunthorpeu8. 10597967656fSJason Gunthorpe 10607967656fSJason GunthorpeSimilarly for function arguments, many true/false values can be consolidated 10617967656fSJason Gunthorpeinto a single bitwise 'flags' argument and 'flags' can often be a more 10627967656fSJason Gunthorpereadable alternative if the call-sites have naked true/false constants. 10637967656fSJason Gunthorpe 10647967656fSJason GunthorpeOtherwise limited use of bool in structures and arguments can improve 10657967656fSJason Gunthorpereadability. 10667967656fSJason Gunthorpe 10677967656fSJason Gunthorpe18) Don't re-invent the kernel macros 1068186128f7SMauro Carvalho Chehab------------------------------------- 1069186128f7SMauro Carvalho Chehab 1070186128f7SMauro Carvalho ChehabThe header file include/linux/kernel.h contains a number of macros that 1071186128f7SMauro Carvalho Chehabyou should use, rather than explicitly coding some variant of them yourself. 1072186128f7SMauro Carvalho ChehabFor example, if you need to calculate the length of an array, take advantage 1073186128f7SMauro Carvalho Chehabof the macro 1074186128f7SMauro Carvalho Chehab 1075186128f7SMauro Carvalho Chehab.. code-block:: c 1076186128f7SMauro Carvalho Chehab 1077186128f7SMauro Carvalho Chehab #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) 1078186128f7SMauro Carvalho Chehab 1079186128f7SMauro Carvalho ChehabSimilarly, if you need to calculate the size of some structure member, use 1080186128f7SMauro Carvalho Chehab 1081186128f7SMauro Carvalho Chehab.. code-block:: c 1082186128f7SMauro Carvalho Chehab 1083c593642cSPankaj Bharadiya #define sizeof_field(t, f) (sizeof(((t*)0)->f)) 1084186128f7SMauro Carvalho Chehab 1085186128f7SMauro Carvalho ChehabThere are also min() and max() macros that do strict type checking if you 1086186128f7SMauro Carvalho Chehabneed them. Feel free to peruse that header file to see what else is already 1087186128f7SMauro Carvalho Chehabdefined that you shouldn't reproduce in your code. 1088186128f7SMauro Carvalho Chehab 1089186128f7SMauro Carvalho Chehab 10907967656fSJason Gunthorpe19) Editor modelines and other cruft 1091186128f7SMauro Carvalho Chehab------------------------------------ 1092186128f7SMauro Carvalho Chehab 1093186128f7SMauro Carvalho ChehabSome editors can interpret configuration information embedded in source files, 1094186128f7SMauro Carvalho Chehabindicated with special markers. For example, emacs interprets lines marked 1095186128f7SMauro Carvalho Chehablike this: 1096186128f7SMauro Carvalho Chehab 1097186128f7SMauro Carvalho Chehab.. code-block:: c 1098186128f7SMauro Carvalho Chehab 1099186128f7SMauro Carvalho Chehab -*- mode: c -*- 1100186128f7SMauro Carvalho Chehab 1101186128f7SMauro Carvalho ChehabOr like this: 1102186128f7SMauro Carvalho Chehab 1103186128f7SMauro Carvalho Chehab.. code-block:: c 1104186128f7SMauro Carvalho Chehab 1105186128f7SMauro Carvalho Chehab /* 1106186128f7SMauro Carvalho Chehab Local Variables: 1107186128f7SMauro Carvalho Chehab compile-command: "gcc -DMAGIC_DEBUG_FLAG foo.c" 1108186128f7SMauro Carvalho Chehab End: 1109186128f7SMauro Carvalho Chehab */ 1110186128f7SMauro Carvalho Chehab 1111186128f7SMauro Carvalho ChehabVim interprets markers that look like this: 1112186128f7SMauro Carvalho Chehab 1113186128f7SMauro Carvalho Chehab.. code-block:: c 1114186128f7SMauro Carvalho Chehab 1115186128f7SMauro Carvalho Chehab /* vim:set sw=8 noet */ 1116186128f7SMauro Carvalho Chehab 1117186128f7SMauro Carvalho ChehabDo not include any of these in source files. People have their own personal 1118186128f7SMauro Carvalho Chehabeditor configurations, and your source files should not override them. This 1119186128f7SMauro Carvalho Chehabincludes markers for indentation and mode configuration. People may use their 1120186128f7SMauro Carvalho Chehabown custom mode, or may have some other magic method for making indentation 1121186128f7SMauro Carvalho Chehabwork correctly. 1122186128f7SMauro Carvalho Chehab 1123186128f7SMauro Carvalho Chehab 11247967656fSJason Gunthorpe20) Inline assembly 1125186128f7SMauro Carvalho Chehab------------------- 1126186128f7SMauro Carvalho Chehab 1127186128f7SMauro Carvalho ChehabIn architecture-specific code, you may need to use inline assembly to interface 1128186128f7SMauro Carvalho Chehabwith CPU or platform functionality. Don't hesitate to do so when necessary. 1129186128f7SMauro Carvalho ChehabHowever, don't use inline assembly gratuitously when C can do the job. You can 1130186128f7SMauro Carvalho Chehaband should poke hardware from C when possible. 1131186128f7SMauro Carvalho Chehab 1132186128f7SMauro Carvalho ChehabConsider writing simple helper functions that wrap common bits of inline 1133186128f7SMauro Carvalho Chehabassembly, rather than repeatedly writing them with slight variations. Remember 1134186128f7SMauro Carvalho Chehabthat inline assembly can use C parameters. 1135186128f7SMauro Carvalho Chehab 1136186128f7SMauro Carvalho ChehabLarge, non-trivial assembly functions should go in .S files, with corresponding 1137186128f7SMauro Carvalho ChehabC prototypes defined in C header files. The C prototypes for assembly 1138186128f7SMauro Carvalho Chehabfunctions should use ``asmlinkage``. 1139186128f7SMauro Carvalho Chehab 1140186128f7SMauro Carvalho ChehabYou may need to mark your asm statement as volatile, to prevent GCC from 1141186128f7SMauro Carvalho Chehabremoving it if GCC doesn't notice any side effects. You don't always need to 1142186128f7SMauro Carvalho Chehabdo so, though, and doing so unnecessarily can limit optimization. 1143186128f7SMauro Carvalho Chehab 1144186128f7SMauro Carvalho ChehabWhen writing a single inline assembly statement containing multiple 1145186128f7SMauro Carvalho Chehabinstructions, put each instruction on a separate line in a separate quoted 114668f04b57SAyan Shafqatstring, and end each string except the last with ``\n\t`` to properly indent 114768f04b57SAyan Shafqatthe next instruction in the assembly output: 1148186128f7SMauro Carvalho Chehab 1149186128f7SMauro Carvalho Chehab.. code-block:: c 1150186128f7SMauro Carvalho Chehab 1151186128f7SMauro Carvalho Chehab asm ("magic %reg1, #42\n\t" 1152186128f7SMauro Carvalho Chehab "more_magic %reg2, %reg3" 1153186128f7SMauro Carvalho Chehab : /* outputs */ : /* inputs */ : /* clobbers */); 1154186128f7SMauro Carvalho Chehab 1155186128f7SMauro Carvalho Chehab 11567967656fSJason Gunthorpe21) Conditional Compilation 1157186128f7SMauro Carvalho Chehab--------------------------- 1158186128f7SMauro Carvalho Chehab 1159186128f7SMauro Carvalho ChehabWherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c 1160186128f7SMauro Carvalho Chehabfiles; doing so makes code harder to read and logic harder to follow. Instead, 1161186128f7SMauro Carvalho Chehabuse such conditionals in a header file defining functions for use in those .c 1162186128f7SMauro Carvalho Chehabfiles, providing no-op stub versions in the #else case, and then call those 1163186128f7SMauro Carvalho Chehabfunctions unconditionally from .c files. The compiler will avoid generating 1164186128f7SMauro Carvalho Chehabany code for the stub calls, producing identical results, but the logic will 1165186128f7SMauro Carvalho Chehabremain easy to follow. 1166186128f7SMauro Carvalho Chehab 1167186128f7SMauro Carvalho ChehabPrefer to compile out entire functions, rather than portions of functions or 1168186128f7SMauro Carvalho Chehabportions of expressions. Rather than putting an ifdef in an expression, factor 1169186128f7SMauro Carvalho Chehabout part or all of the expression into a separate helper function and apply the 1170186128f7SMauro Carvalho Chehabconditional to that function. 1171186128f7SMauro Carvalho Chehab 1172186128f7SMauro Carvalho ChehabIf you have a function or variable which may potentially go unused in a 1173186128f7SMauro Carvalho Chehabparticular configuration, and the compiler would warn about its definition 1174186128f7SMauro Carvalho Chehabgoing unused, mark the definition as __maybe_unused rather than wrapping it in 1175186128f7SMauro Carvalho Chehaba preprocessor conditional. (However, if a function or variable *always* goes 1176186128f7SMauro Carvalho Chehabunused, delete it.) 1177186128f7SMauro Carvalho Chehab 1178186128f7SMauro Carvalho ChehabWithin code, where possible, use the IS_ENABLED macro to convert a Kconfig 1179186128f7SMauro Carvalho Chehabsymbol into a C boolean expression, and use it in a normal C conditional: 1180186128f7SMauro Carvalho Chehab 1181186128f7SMauro Carvalho Chehab.. code-block:: c 1182186128f7SMauro Carvalho Chehab 1183186128f7SMauro Carvalho Chehab if (IS_ENABLED(CONFIG_SOMETHING)) { 1184186128f7SMauro Carvalho Chehab ... 1185186128f7SMauro Carvalho Chehab } 1186186128f7SMauro Carvalho Chehab 1187186128f7SMauro Carvalho ChehabThe compiler will constant-fold the conditional away, and include or exclude 1188186128f7SMauro Carvalho Chehabthe block of code just as with an #ifdef, so this will not add any runtime 1189186128f7SMauro Carvalho Chehaboverhead. However, this approach still allows the C compiler to see the code 1190186128f7SMauro Carvalho Chehabinside the block, and check it for correctness (syntax, types, symbol 1191186128f7SMauro Carvalho Chehabreferences, etc). Thus, you still have to use an #ifdef if the code inside the 1192186128f7SMauro Carvalho Chehabblock references symbols that will not exist if the condition is not met. 1193186128f7SMauro Carvalho Chehab 1194186128f7SMauro Carvalho ChehabAt the end of any non-trivial #if or #ifdef block (more than a few lines), 1195186128f7SMauro Carvalho Chehabplace a comment after the #endif on the same line, noting the conditional 1196186128f7SMauro Carvalho Chehabexpression used. For instance: 1197186128f7SMauro Carvalho Chehab 1198186128f7SMauro Carvalho Chehab.. code-block:: c 1199186128f7SMauro Carvalho Chehab 1200186128f7SMauro Carvalho Chehab #ifdef CONFIG_SOMETHING 1201186128f7SMauro Carvalho Chehab ... 1202186128f7SMauro Carvalho Chehab #endif /* CONFIG_SOMETHING */ 1203186128f7SMauro Carvalho Chehab 1204186128f7SMauro Carvalho Chehab 12051cfd9d7eSDavid Hildenbrand22) Do not crash the kernel 12061cfd9d7eSDavid Hildenbrand--------------------------- 12071cfd9d7eSDavid Hildenbrand 12081cfd9d7eSDavid HildenbrandIn general, the decision to crash the kernel belongs to the user, rather 12091cfd9d7eSDavid Hildenbrandthan to the kernel developer. 12101cfd9d7eSDavid Hildenbrand 12111cfd9d7eSDavid HildenbrandAvoid panic() 12121cfd9d7eSDavid Hildenbrand************* 12131cfd9d7eSDavid Hildenbrand 12141cfd9d7eSDavid Hildenbrandpanic() should be used with care and primarily only during system boot. 12151cfd9d7eSDavid Hildenbrandpanic() is, for example, acceptable when running out of memory during boot and 12161cfd9d7eSDavid Hildenbrandnot being able to continue. 12171cfd9d7eSDavid Hildenbrand 12181cfd9d7eSDavid HildenbrandUse WARN() rather than BUG() 12191cfd9d7eSDavid Hildenbrand**************************** 12201cfd9d7eSDavid Hildenbrand 12211cfd9d7eSDavid HildenbrandDo not add new code that uses any of the BUG() variants, such as BUG(), 12221cfd9d7eSDavid HildenbrandBUG_ON(), or VM_BUG_ON(). Instead, use a WARN*() variant, preferably 12231cfd9d7eSDavid HildenbrandWARN_ON_ONCE(), and possibly with recovery code. Recovery code is not 12241cfd9d7eSDavid Hildenbrandrequired if there is no reasonable way to at least partially recover. 12251cfd9d7eSDavid Hildenbrand 12261cfd9d7eSDavid Hildenbrand"I'm too lazy to do error handling" is not an excuse for using BUG(). Major 12271cfd9d7eSDavid Hildenbrandinternal corruptions with no way of continuing may still use BUG(), but need 12281cfd9d7eSDavid Hildenbrandgood justification. 12291cfd9d7eSDavid Hildenbrand 12301cfd9d7eSDavid HildenbrandUse WARN_ON_ONCE() rather than WARN() or WARN_ON() 12311cfd9d7eSDavid Hildenbrand************************************************** 12321cfd9d7eSDavid Hildenbrand 12331cfd9d7eSDavid HildenbrandWARN_ON_ONCE() is generally preferred over WARN() or WARN_ON(), because it 12341cfd9d7eSDavid Hildenbrandis common for a given warning condition, if it occurs at all, to occur 12351cfd9d7eSDavid Hildenbrandmultiple times. This can fill up and wrap the kernel log, and can even slow 12361cfd9d7eSDavid Hildenbrandthe system enough that the excessive logging turns into its own, additional 12371cfd9d7eSDavid Hildenbrandproblem. 12381cfd9d7eSDavid Hildenbrand 12391cfd9d7eSDavid HildenbrandDo not WARN lightly 12401cfd9d7eSDavid Hildenbrand******************* 12411cfd9d7eSDavid Hildenbrand 12421cfd9d7eSDavid HildenbrandWARN*() is intended for unexpected, this-should-never-happen situations. 12431cfd9d7eSDavid HildenbrandWARN*() macros are not to be used for anything that is expected to happen 12441cfd9d7eSDavid Hildenbrandduring normal operation. These are not pre- or post-condition asserts, for 12451cfd9d7eSDavid Hildenbrandexample. Again: WARN*() must not be used for a condition that is expected 12461cfd9d7eSDavid Hildenbrandto trigger easily, for example, by user space actions. pr_warn_once() is a 12471cfd9d7eSDavid Hildenbrandpossible alternative, if you need to notify the user of a problem. 12481cfd9d7eSDavid Hildenbrand 12491cfd9d7eSDavid HildenbrandDo not worry about panic_on_warn users 12501cfd9d7eSDavid Hildenbrand************************************** 12511cfd9d7eSDavid Hildenbrand 12521cfd9d7eSDavid HildenbrandA few more words about panic_on_warn: Remember that ``panic_on_warn`` is an 12531cfd9d7eSDavid Hildenbrandavailable kernel option, and that many users set this option. This is why 12541cfd9d7eSDavid Hildenbrandthere is a "Do not WARN lightly" writeup, above. However, the existence of 12551cfd9d7eSDavid Hildenbrandpanic_on_warn users is not a valid reason to avoid the judicious use 12561cfd9d7eSDavid HildenbrandWARN*(). That is because, whoever enables panic_on_warn has explicitly 12571cfd9d7eSDavid Hildenbrandasked the kernel to crash if a WARN*() fires, and such users must be 12581cfd9d7eSDavid Hildenbrandprepared to deal with the consequences of a system that is somewhat more 12591cfd9d7eSDavid Hildenbrandlikely to crash. 12601cfd9d7eSDavid Hildenbrand 12611cfd9d7eSDavid HildenbrandUse BUILD_BUG_ON() for compile-time assertions 12621cfd9d7eSDavid Hildenbrand********************************************** 12631cfd9d7eSDavid Hildenbrand 12641cfd9d7eSDavid HildenbrandThe use of BUILD_BUG_ON() is acceptable and encouraged, because it is a 12651cfd9d7eSDavid Hildenbrandcompile-time assertion that has no effect at runtime. 12661cfd9d7eSDavid Hildenbrand 1267186128f7SMauro Carvalho ChehabAppendix I) References 1268186128f7SMauro Carvalho Chehab---------------------- 1269186128f7SMauro Carvalho Chehab 1270186128f7SMauro Carvalho ChehabThe C Programming Language, Second Edition 1271186128f7SMauro Carvalho Chehabby Brian W. Kernighan and Dennis M. Ritchie. 1272186128f7SMauro Carvalho ChehabPrentice Hall, Inc., 1988. 1273186128f7SMauro Carvalho ChehabISBN 0-13-110362-8 (paperback), 0-13-110370-9 (hardback). 1274186128f7SMauro Carvalho Chehab 1275186128f7SMauro Carvalho ChehabThe Practice of Programming 1276186128f7SMauro Carvalho Chehabby Brian W. Kernighan and Rob Pike. 1277186128f7SMauro Carvalho ChehabAddison-Wesley, Inc., 1999. 1278186128f7SMauro Carvalho ChehabISBN 0-201-61586-X. 1279186128f7SMauro Carvalho Chehab 1280186128f7SMauro Carvalho ChehabGNU manuals - where in compliance with K&R and this text - for cpp, gcc, 1281e7b4311eSAlexander A. Klimovgcc internals and indent, all available from https://www.gnu.org/manual/ 1282186128f7SMauro Carvalho Chehab 1283186128f7SMauro Carvalho ChehabWG14 is the international standardization working group for the programming 1284186128f7SMauro Carvalho Chehablanguage C, URL: http://www.open-std.org/JTC1/SC22/WG14/ 1285186128f7SMauro Carvalho Chehab 1286775a445dSJakub WilkKernel CodingStyle, by [email protected] at OLS 2002: 1287186128f7SMauro Carvalho Chehabhttp://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/ 1288