diff options
author | Mark Whitley <markw@lineo.com> | 2000-11-22 19:25:39 +0000 |
---|---|---|
committer | Mark Whitley <markw@lineo.com> | 2000-11-22 19:25:39 +0000 |
commit | d58ff8731ce635a75406cc8b9629f1041bb4ed32 (patch) | |
tree | f9bbfd3e14d3f884af2b8c28e747c260e2b01e16 /docs/style-guide.txt | |
parent | 9a71af54f53332dda41823763d2ea85a4afdf2e0 (diff) | |
download | busybox-w32-d58ff8731ce635a75406cc8b9629f1041bb4ed32.tar.gz busybox-w32-d58ff8731ce635a75406cc8b9629f1041bb4ed32.tar.bz2 busybox-w32-d58ff8731ce635a75406cc8b9629f1041bb4ed32.zip |
Re-organized some sections and added a whole new section on avoiding the
preprocessor. Comments welcome.
Diffstat (limited to 'docs/style-guide.txt')
-rw-r--r-- | docs/style-guide.txt | 262 |
1 files changed, 204 insertions, 58 deletions
diff --git a/docs/style-guide.txt b/docs/style-guide.txt index 36974d7f5..9ea236024 100644 --- a/docs/style-guide.txt +++ b/docs/style-guide.txt | |||
@@ -16,6 +16,7 @@ right formatting rules to your file. Please _do_not_ run this on all the files | |||
16 | in the directory, just your own. | 16 | in the directory, just your own. |
17 | 17 | ||
18 | 18 | ||
19 | |||
19 | Declaration Order | 20 | Declaration Order |
20 | ----------------- | 21 | ----------------- |
21 | 22 | ||
@@ -31,15 +32,16 @@ Here is the order in which code should be laid out in a file: | |||
31 | - function implementations | 32 | - function implementations |
32 | 33 | ||
33 | 34 | ||
34 | Whitespace | 35 | |
35 | ---------- | 36 | Whitespace and Formatting |
37 | ------------------------- | ||
36 | 38 | ||
37 | This is everybody's favorite flame topic so let's get it out of the way right | 39 | This is everybody's favorite flame topic so let's get it out of the way right |
38 | up front. | 40 | up front. |
39 | 41 | ||
40 | 42 | ||
41 | Tabs vs. Spaces in Line Indentation | 43 | Tabs vs. Spaces in Line Indentation |
42 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | 44 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
43 | 45 | ||
44 | The preference in Busybox is to indent lines with tabs. Do not indent lines | 46 | The preference in Busybox is to indent lines with tabs. Do not indent lines |
45 | with spaces and do not indents lines using a mixture of tabs and spaces. (The | 47 | with spaces and do not indents lines using a mixture of tabs and spaces. (The |
@@ -172,6 +174,7 @@ block. Example: | |||
172 | } | 174 | } |
173 | 175 | ||
174 | 176 | ||
177 | |||
175 | Variable and Function Names | 178 | Variable and Function Names |
176 | --------------------------- | 179 | --------------------------- |
177 | 180 | ||
@@ -192,78 +195,195 @@ that can go through and convert files -- left as an exercise to the reader for | |||
192 | now. | 195 | now. |
193 | 196 | ||
194 | 197 | ||
195 | Tip and Pointers | ||
196 | ---------------- | ||
197 | 198 | ||
198 | The following are simple coding guidelines that should be followed: | 199 | Avoid The Preprocessor |
200 | ---------------------- | ||
199 | 201 | ||
200 | - When in doubt about the proper behavior of a Busybox program (output, | 202 | At best, the preprocessor is a necessary evil, helping us account for platform |
201 | formatting, options, etc.), model it after the equivalent GNU program. | 203 | and architecture differences. Using the preprocessor unnecessarily is just |
202 | Doesn't matter how that program behaves on some other flavor of *NIX; | 204 | plain evil. |
203 | doesn't matter what the POSIX standard says or doesn't say, just model | ||
204 | Busybox programs after their GNU counterparts and nobody has to get hurt. | ||
205 | 205 | ||
206 | - Don't use a '#define var 80' when you can use 'static const int var 80' | ||
207 | instead. This makes the compiler do type checking for you (rather than | ||
208 | relying on the more error-prone preprocessor) and it makes debugging | ||
209 | programs much easier since the value of the variable can be easily | ||
210 | displayed. | ||
211 | 206 | ||
212 | - If a const variable is used in only one function, do not make it global to | 207 | The Folly of #define |
213 | the file. Instead, declare it inside the function body. | 208 | ~~~~~~~~~~~~~~~~~~~~ |
214 | 209 | ||
215 | - Inside applet files, all functions should be declared static so as to keep | 210 | Use 'const <type> var' for declaring constants. |
216 | the global name space clean. The only exception to this rule is the | ||
217 | "applet_main" function which must be declared extern. | ||
218 | 211 | ||
219 | - If you write a function that performs a task that could be useful outside | 212 | Don't do this: |
220 | the immediate file, turn it into a general-purpose function with no ties to | ||
221 | any applet and put it in the utility.c file instead. | ||
222 | 213 | ||
223 | - Put all help/usage messages in usage.c. Put other strings in messages.c. | 214 | #define var 80 |
224 | Putting these strings into their own file is a calculated decision designed | ||
225 | to confine spelling errors to a single place and aid internationalization | ||
226 | efforts, if needed. (Side Note: we might want to use a single file instead | ||
227 | of two, food for thought). | ||
228 | 215 | ||
229 | - There's a right way and a wrong way to test for sting equivalence with | 216 | Do this instead, when the variable is in a header file and will be used in |
230 | strcmp: | 217 | several source files: |
231 | 218 | ||
232 | The wrong way: | 219 | const int var = 80; |
233 | 220 | ||
234 | if (!strcmp(string, "foo")) { | 221 | Or do this when the variable is used only in a single source file: |
235 | ... | ||
236 | 222 | ||
237 | The right way: | 223 | static const int var = 80; |
238 | 224 | ||
239 | if (strcmp(string, "foo") == 0){ | 225 | Declaring variables as '[static] const' gives variables an actual type and |
240 | ... | 226 | makes the compiler do type checking for you; the preprocessor does _no_ type |
227 | checking whatsoever, making it much more error prone. Declaring variables with | ||
228 | '[static] const' also makes debugging programs much easier since the value of | ||
229 | the variable can be easily queried and displayed. | ||
241 | 230 | ||
242 | The use of the "equals" (==) operator in the latter example makes it much | ||
243 | more obvious that you are testing for equivalence. The former example with | ||
244 | the "not" (!) operator makes it look like you are testing for an error. In | ||
245 | a more perfect world, we would have a streq() function in the string | ||
246 | library, but that ain't the world we're living in. | ||
247 | 231 | ||
248 | - Do not use old-style function declarations that declare variable types | 232 | The Folly of Macros |
249 | between the parameter list and opening bracket. Example: | 233 | ~~~~~~~~~~~~~~~~~~~ |
234 | |||
235 | Use 'static inline' instead of a macro. | ||
250 | 236 | ||
251 | Don't do this: | 237 | Don't do this: |
252 | 238 | ||
253 | int foo(parm1, parm2) | 239 | #define mini_func(param1, param2) (param1 << param2) |
254 | char parm1; | ||
255 | float parm2; | ||
256 | { | ||
257 | .... | ||
258 | 240 | ||
259 | Do this instead: | 241 | Do this instead: |
260 | 242 | ||
261 | int foo(char parm1, float parm2) | 243 | static inline int mini_func(int param1, param2) |
262 | { | 244 | { |
263 | .... | 245 | return (param1 << param2); |
246 | } | ||
247 | |||
248 | Static inline functions are greatly preferred over macros. They provide type | ||
249 | safety, have no length limitations, no formatting limitations, and under gcc | ||
250 | they are as cheap as macros. Besides, really long macros with backslashes at | ||
251 | the end of each line are ugly as sin. | ||
252 | |||
264 | 253 | ||
265 | - Please use brackets on all if and else statements, even if it is only one | 254 | The Folly of #ifdef |
266 | line. Example: | 255 | ~~~~~~~~~~~~~~~~~~~ |
256 | |||
257 | Code cluttered with ifdefs is difficult to read and maintain. Don't do it. | ||
258 | Instead, put your ifdefs in a header, and conditionally define 'static inline' | ||
259 | functions, (or *maybe* macros), which are used in the code. | ||
260 | |||
261 | Don't do this: | ||
262 | |||
263 | ret = my_func(bar, baz); | ||
264 | if (!ret) | ||
265 | return -1; | ||
266 | #ifdef BB_FEATURE_FUNKY | ||
267 | maybe_do_funky_stuff(bar, baz); | ||
268 | #endif | ||
269 | |||
270 | Do this instead: | ||
271 | |||
272 | (in .h header file) | ||
273 | |||
274 | #ifndef BB_FEATURE_FUNKY | ||
275 | static inline void maybe_do_funky_stuff (int bar, int baz) {} | ||
276 | #endif | ||
277 | |||
278 | (in the .c source file) | ||
279 | |||
280 | ret = my_func(bar, baz); | ||
281 | if (!ret) | ||
282 | return -1; | ||
283 | maybe_do_funky_stuff(bar, baz); | ||
284 | |||
285 | The great thing about this approach is that the compiler will optimize away | ||
286 | the "no-op" case when the feature is turned off. | ||
287 | |||
288 | Note also the use of the word 'maybe' in the function name to indicate | ||
289 | conditional execution. | ||
290 | |||
291 | |||
292 | |||
293 | Notes on Strings | ||
294 | ---------------- | ||
295 | |||
296 | Strings in C can get a little thorny. Here's some guidelines for dealing with | ||
297 | strings in Busybox. (There is surely more that could be added to this | ||
298 | section.) | ||
299 | |||
300 | |||
301 | String Files | ||
302 | ~~~~~~~~~~~~ | ||
303 | |||
304 | Put all help/usage messages in usage.c. Put other strings in messages.c. | ||
305 | Putting these strings into their own file is a calculated decision designed to | ||
306 | confine spelling errors to a single place and aid internationalization | ||
307 | efforts, if needed. (Side Note: we might want to use a single file - maybe | ||
308 | called 'strings.c' - instead of two, food for thought). | ||
309 | |||
310 | |||
311 | Testing String Equivalence | ||
312 | ~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
313 | |||
314 | There's a right way and a wrong way to test for sting equivalence with | ||
315 | strcmp(): | ||
316 | |||
317 | The wrong way: | ||
318 | |||
319 | if (!strcmp(string, "foo")) { | ||
320 | ... | ||
321 | |||
322 | The right way: | ||
323 | |||
324 | if (strcmp(string, "foo") == 0){ | ||
325 | ... | ||
326 | |||
327 | The use of the "equals" (==) operator in the latter example makes it much more | ||
328 | obvious that you are testing for equivalence. The former example with the | ||
329 | "not" (!) operator makes it look like you are testing for an error. In a more | ||
330 | perfect world, we would have a streq() function in the string library, but | ||
331 | that ain't the world we're living in. | ||
332 | |||
333 | |||
334 | |||
335 | Miscellaneous Coding Guidelines | ||
336 | ------------------------------- | ||
337 | |||
338 | The following are important items that don't fit into any of the above | ||
339 | sections. | ||
340 | |||
341 | |||
342 | Model Busybox Applets After GNU Counterparts | ||
343 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
344 | |||
345 | When in doubt about the proper behavior of a Busybox program (output, | ||
346 | formatting, options, etc.), model it after the equivalent GNU program. | ||
347 | Doesn't matter how that program behaves on some other flavor of *NIX; doesn't | ||
348 | matter what the POSIX standard says or doesn't say, just model Busybox | ||
349 | programs after their GNU counterparts and nobody has to get hurt. | ||
350 | |||
351 | The only time we deviate from emulating the GNU behavior is when: | ||
352 | |||
353 | - We are deliberately not supporting a feature (such as a command line | ||
354 | switch) | ||
355 | - Emulating the GNU behavior is prohibitively expensive (lots more code | ||
356 | would be required, lots more memory would be used, etc.) | ||
357 | - The differce is minor or cosmetic | ||
358 | |||
359 | A note on the 'cosmetic' case: Output differences might be considered | ||
360 | cosmetic, but if the output is significant enough to break other scripts that | ||
361 | use the output, it should really be fixed. | ||
362 | |||
363 | |||
364 | Scope | ||
365 | ~~~~~ | ||
366 | |||
367 | If a const variable is used only in a single source file, put it in the source | ||
368 | file and not in a header file. Likewise, if a const variable is used in only | ||
369 | one function, do not make it global to the file. Instead, declare it inside | ||
370 | the function body. Bottom line: Make a concious effort to limit declarations | ||
371 | to the smallest scope possible. | ||
372 | |||
373 | Inside applet files, all functions should be declared static so as to keep the | ||
374 | global name space clean. The only exception to this rule is the "applet_main" | ||
375 | function which must be declared extern. | ||
376 | |||
377 | If you write a function that performs a task that could be useful outside the | ||
378 | immediate file, turn it into a general-purpose function with no ties to any | ||
379 | applet and put it in the utility.c file instead. | ||
380 | |||
381 | |||
382 | Brackets Are Your Friends | ||
383 | ~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
384 | |||
385 | Please use brackets on all if and else statements, even if it is only one | ||
386 | line. Example: | ||
267 | 387 | ||
268 | Don't do this: | 388 | Don't do this: |
269 | 389 | ||
@@ -280,8 +400,8 @@ The following are simple coding guidelines that should be followed: | |||
280 | stmt; | 400 | stmt; |
281 | } | 401 | } |
282 | 402 | ||
283 | The "bracketless" approach is error prone because someday you might add a | 403 | The "bracketless" approach is error prone because someday you might add a line |
284 | line like this: | 404 | like this: |
285 | 405 | ||
286 | if (foo) | 406 | if (foo) |
287 | stmt; | 407 | stmt; |
@@ -289,6 +409,32 @@ The following are simple coding guidelines that should be followed: | |||
289 | else | 409 | else |
290 | stmt; | 410 | stmt; |
291 | 411 | ||
292 | And the resulting behavior of your program would totally bewilder you. | 412 | And the resulting behavior of your program would totally bewilder you. (Don't |
293 | (Don't laugh, it happens to us all.) Remember folks, this is C, not | 413 | laugh, it happens to us all.) Remember folks, this is C, not Python. |
294 | Python. | 414 | |
415 | |||
416 | Function Declarations | ||
417 | ~~~~~~~~~~~~~~~~~~~~~ | ||
418 | |||
419 | Do not use old-style function declarations that declare variable types between | ||
420 | the parameter list and opening bracket. Example: | ||
421 | |||
422 | Don't do this: | ||
423 | |||
424 | int foo(parm1, parm2) | ||
425 | char parm1; | ||
426 | float parm2; | ||
427 | { | ||
428 | .... | ||
429 | |||
430 | Do this instead: | ||
431 | |||
432 | int foo(char parm1, float parm2) | ||
433 | { | ||
434 | .... | ||
435 | |||
436 | The only time you would ever need to use the old declaration syntax is to | ||
437 | support ancient, antedeluvian compilers. To our good fortune, we have access | ||
438 | to more modern compilers and the old declaration syntax is neither necessary | ||
439 | nor desired. | ||
440 | |||