|
Message-ID: <790eac282623f7a30dac43ff945ce6c0@smtp.hushmail.com> Date: Sat, 25 Jul 2015 16:41:19 +0200 From: magnum <john.magnum@...hmail.com> To: john-dev@...ts.openwall.com Subject: Re: Coding Style On 2015-07-25 15:05, Kai Zhao wrote: > There are some rules that some people like while others don't. I am very > glad to get your advice for each rules whether agree or disagree.Thus, I > can finish this coding style documentation and there are less coding style > problems. Here are my two cents. In general I think we should should have a terse and liberal code style, allowing some variations (details below). > Chapter 1: Indentation > > 1.1 Tabs are 4 characters for jumbo (8 for core), and thus indentations are > also 4 characters. Regardless of what width you use, indentation one (1) tab (per level). To me, mentioning a number of characters for indentation is strange and incorrectly suggests spaces can be used. I would put it eg. like this: 1.1 Indentation is one tab per level. Recommended tab width is 4 or 8 for jumbo and 8 for core but it mostly just affects where a line exceeds max length. > 1.2 Indent with tabs, align with spaces. E.g. OK. > 1.3 Ease multiple indentation levels in switch(), for(), while()... OK. > 1.4 Don't put multiple statements on a single line. A good example is: OK. > 1.5 Don't put multiple assignments on a single line either. > > Do't do this: > > i = j = pos = -1; IMHO we should allow this. > or > > *pos++ = 0; *ptr = pos; OK, to me that's repeating 1.4. > Chapter 2: Breaking long lines and strings > > The limit on the length of lines is 80 columns. > > However, there are some cases that the lines can exceed 80 columns. > Never break user-visible strings such as printk messages, because > that breaks the ability to grep for them. We should mention something other than 'printk'. > Chapter 3: Placing Braces and Spaces > > 3.1 Braces > > 3.1.1 Function > > Put the opening brace at the beginning of the next line, thus: OK. > 3.1.2 Others > > Put the opening brace last on the next line, thus: > (...) > > Note that the closing brace is empty on a line of its own, _except_ in > the cases where it is followed by a continuation of the same statement, > ie a "while" in a do-statement or an "else" in an if-statement, like > this: > > do { > body of do-loop > } while (condition); > > and > > if (x == y) { > .. > } else if (x > y) { > ... > } else { > .... > } IMHO we should allow this too if (x == y) { .. } else if (x > y) { ... } else { .... } > 3.1.3 Do not unnecessarily use braces where a single statement will do. I think we should allow this. > 3.2 Spaces > > 3.2.1 Use a space after (most) keywords. OK. > 3.2.2 Do not add spaces around (inside) parenthesized expressions. > > This example is *bad*: > > s = sizeof( struct file ); I never write it like that but maybe we should be more liberal here. > 3.2.3 When declaring pointer, the preferred use of '*' is adjacent to > the data name or function name. E.g.: > > char *linux_banner; > unsigned long long memparse(char *ptr, char **retptr); > char *match_strdup(substring_t *s); OK. > 3.2.4 When type conversion, add a space before '*'. > > E.g: > > hostSalt = (cl_uint *) mem_alloc(SALT_BUFFER_SIZE); I personally disagree. We should mandate NOT adding a space, or just be liberal about it. Actually I always drop the space after the paren too: hostSalt = (cl_uint*)mem_alloc(SALT_BUFFER_SIZE); I think we should allow (not require) a space after the paren, and either allow or require that there's space before the '*'. > 3.2.5 Use one space around (on each side of) most binary and ternary operators, > such as any of these: OK. > 3.2.6 Don't leave whilespace at the end of lines. OK. > Chapter 4: Naming > > 4.1 Names for global variables and global functions > > GLOBAL variables (to be used only if you _really_ need them) need to > have descriptive names, as do global functions. If you have a function > that counts the number of active users, you should call that > "count_active_users()" or similar, you should _not_ call it "cntusr()". Also: We use names prefixed by crk_ for global functions in cracker.c, ldr_ for ones from loader.c and so on. Not sure how we would express that though. > 4.2 Names for local variables > > LOCAL variable names should be short, and to the point. If you have > some random integer loop counter, it should probably be called "i". > Calling it "loop_counter" is non-productive, if there is no chance of it > being mis-understood. Similarly, "tmp" can be just about any type of > variable that is used to hold a temporary value. I think we should drop the above even though I agree with it. We should be liberal. We could mandate against using camelCase but I'm not sure we should do that (for Jumbo) either. > Chapter 5: Typedefs > > NOTE: This chapter is totally from Kernel CodingStyle. I think many codes > of john do't follow this rule. So should we follow this rule ? > > Please don't use things like "vps_t". > It's a _mistake_ to use typedef for structures and pointers. When you see a For our code, I disagree. > Chapter 6: Functions Our code style should not be a programming tutorial. IMHO drop all of chapter 6. > Chapter 7: Centralized exiting of functions Same here. > Chapter 8: Commenting > > 8.1 C89 style and C99 style > > John core and Linux style for comments is the C89 /* ... */ I'm not overly happy about mandating this in Jumbo. > 8.2 Comment content Our code style should not be a programming tutorial. IMHO drop this. > Chapter 9: Macros, Enums > > 9.1 Names of macros defining constants and labels in enums are capitalized. > > #define CONSTANT 0x12345 > > Enums are preferred when defining several related constants. > > CAPITALIZED macro names are appreciated but macros resembling functions > may be named in lower case. OK. I'm not sure we need this although I agree with it. > Generally, inline functions are preferable to macros resembling functions. Maybe drop this. > 9.2 Macros with multiple statements should be enclosed in a do - while block: > > #define macrofun(a, b, c) \ > do { \ > if (a == 5) \ > do_this(b, c); \ > } while (0) This is going towards "tutorial" again and I dislike that. > 9.3 Things to avoid when using macros: Same here. These are good advices but I think they have no place in our coding style. > Chapter 10: The inline disease > > A reasonable rule of thumb is to not put inline at functions that have more > than 3 lines of code in them. An exception to this rule are the cases where > a parameter is known to be a compiletime constant, and as a result of this > constantness you *know* the compiler will be able to optimize most of your > function away at compile time. While this is probably reasonable advice, I think we don't need it in our code style. > Chapter 11: Function return values and names I think this does not apply to JtR at all. magnum
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.