From 1e5d9338ffd21f614cc291e9c1942cec93dce377 Mon Sep 17 00:00:00 2001 From: _Felix_ <_Felix_@80855669-be73-4f5e-9bfa-13ee22678a59> Date: Wed, 6 Jun 2012 22:02:19 +0000 Subject: [PATCH] Added the uOFW coding style standard ("Coding Style Standard"). --- Coding Style Standard | 393 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 393 insertions(+) create mode 100644 Coding Style Standard diff --git a/Coding Style Standard b/Coding Style Standard new file mode 100644 index 0000000..da30d04 --- /dev/null +++ b/Coding Style Standard @@ -0,0 +1,393 @@ +This document is about the preferred uOFW coding style. Please stick to the style +presented in this guide to make working on uOFW as a team easier. +Thank you. + +1) Indentation: + +Indentations are 4 bytes wide. The reason behind this is to visibly show +where a block starts and where it ends. + +Example: + +if (condition) { + doX(); + doY(); +} + +Don't put multiple statements on a single line: + +/* bad */ +if (condition) doX(); + +For switch blocks, don't indent the "case"s and "default"s but only their content. +In example: + +switch (type) { +case 'A': + break; +} + +2) Breaking long lines: + +Make sure to not write code lines longer than 120 characters. If neccessary, break lines exceeding the 120 +characters limit like below: + +SCE_MODULE_INFO("sceController_Service", SCE_MODULE_KERNEL | SCE_MODULE_NO_STOP | SCE_MODULE_SINGLE_LOAD | + SCE_MODULE_SINGLE_START, 1, 1); + +This applies to functions as well. + +Long if-lines (same as while-, for-lines) are broken into several lines like this: + +if (a == 1 || + b == 2) + doSomething(); + + +3) Placing Braces and Spaces: + +Put the opening brace last on the line, and put the closing brace first, thusly: + +if (condition) { + doX(); + doY(); +} + +This applies to all non-function code blocks (if, switch, for, while). +Futhermore it is used when using typdef or array/structure initialization. + +However, don't put braces where only one branch exists with just one single +statement. +Reason: The indentation is already visible and thus there is no need for braces. + +if (condition) + doX(); + +If there are two or more branches, use braces this way: + +if (condition) { + doX(); + doZ(); +} else if (otherCondition) { + doA(); + doB(); +} else { + doY(); + doW(); +} + +And don't put braces when the block contents are only one line long, the same way +as above: + +if (condition) { + doX(); + doZ(); +} else if (otherCondition) + doY(); +else + doW(); + +For functions, put the opening brace first on a new line and the closing brace +first on the last line: + +void function(void) +{ + doSomethingCool(); +} + +Space policy: + +Put a space after these keywords: if, switch, case, do, while, for, return. + +switch (number) { +case 1: // I think we don't need the double-indentation here; I would put the "case" at the same indentation level than the switch, I think Linux uses that + doThis(); + break; +case 2: + doThat(); + break; +default: + doStandard(); + break; +} + +Don't put spaces after keywords not mentioned above. + +Put spaces before and after arithmetic operators (+, -, *, /, %), shift operators, +binary operators (&, |, ^), comparison & operation operators. + +Example: + +if ((a + (b << 2)) > 4) + doSomeMagic(); + +Consequently, don't put spaces before and after prefix/postfix operators (++/--), +or after unary operators like &, *, !, ~. +Furthermore, no spaces around structure member operators (. and ->). + +The sizeof compile-time unary operator is special. Use a space after it only when +taking the size of an object, don't use a space (but parantheses!) when taking +the size of a data type. + +sizeof object; -> (sizeof variable;) + +sizeof(type name); -> (sizeof(u32);) + +Declaring a pointer is made by adjoining the unary operator '*' to the data name +and not the type name: + +s8 *ptr; + +When declaring a pointer to a function, this way suffices: + +s32 (*funcPtr)(void); + +4) Type using: + +Don't use type's like (unsigned) short, (unsigned) int as their size is NOT +the same on all machines. +Use instead: + +8 bit signed/unsigned data size -> s8/u8 +16 bit signed/unsigned data size -> s16/u16 +32 bit signed/unsigned data size -> s32/u32 +64 bit signed/unsigned data size -> s64/u64 + +s8 applies to arrays of characters as well, obviously: + +s8 project[] = "uOFW"; + +Also use these data types for casts, when necessary. + +When declaring a structure type using a typedef, do it this way: + +typedef struct { + s32 x; + s32 y; +} Point; + +and not: + +/* bad */ +typedef struct _Point { + s32 x; + s32 y; +} Point; + +Except if defining the structure itself is needed, in example if the structure references itself. +In that case, use: +typedef struct Point { + struct Point *next; +} Point; + +5) Naming: + +Use simple, short names, easy to understand. For local variables, it is even okay +to use a variable name consisting of only one letter: + +/* bad */ +u8 loopCounter; + +/* good */ +u8 i; + +Another common example is the use of temporary variables. Just name them "tmp", +this is clearly understandable and does its job. Don't try making the program +harder to understand than neccessary. + +If you encounter a local variable and you don't know what it represents, name +it "unk". If there are more unknown variables within one function, name them +like this: + +s32 unk1; +s32 unk2; +... + +This applies for unknown structure members too. Note that if there are a few +different structures in the same file, using different "styles" (like unk1 or +unk001 or unk_1..) can be useful so the member name can be replaced easily in all +the file. + +For global variables, use descriptive names. Here, the variables' names +matter and not primarily the name length. +In order to clearly distinguish global variables from local variables +(especially helpfull in large functions), use a "g_" prefix for global's. + +/* global mutexId */ +s32 g_MutexId; + +Don't use underscores in variable names, except for the "g_" prefix for globals. +Instead, use capital letters. + +/* Bad */ +u32 is_uofw_cool; + +/* Good */ +u32 isUofwCool = 1; /* Indeed. */ + +6) Don't use magic numbers! + +Using magic numbers is bad, as only the author of the C file in which they occur +will know (for a little while) what they express. +Instead, use a #define statement or an enumeration to give numbers a meaningful name. + +The approach is to give hardware important #defines a "PSP_" prefix and general #defines shared with +other files a "SCE_" prefix, as in "SCE_MODULE_KERNEL". + +Hardware-important example #define from ctrl.c: + +/* This is bad, what SYSCON transmit command does 7 represent? */ +g_Ctrl.sysPacket[0].tx_cmd = 7; + +Thus, we have the following #define in syscon.h: +#define PSP_SYSCON_CMD_GET_KERNEL_DIGITAL_KEY 7 + +/* Aha, "7" stands for a data tranfer of all digital buttons (no analog pad data). */ +ctrl.sysPacket[0].tx_cmd = PSP_SYSCON_CMD_GET_KERNEL_DIGITAL_KEY; + +The same goes for return codes. uOFW's error file is located in +/uOFW/trunk/include/common/error.h. Use the defined errors instead of magic numbers. + +A similar approach for for/while-loops: + +/* This is bad code */ +u32 g_Array[10]; + +u32 i; +for (i = 0; i < 10; i++) + g_Array[i] = 0; + +/* Good code */ +#define ARRAY_SIZE 10 + +u32 g_Array[ARRAY_SIZE]; + +u32 i; +for (i = 0; i < ARRAY_SIZE; i++) + g_Array[i] = 0; + + +7) Structure initialization: + +Assume we have the following structure: + +typedef struct { + s32 x; + s32 y; +} Point; + +The initialization of a Point variable looks like this: + +Point p = { + .x = 20, + .y = 12 +}; + +Directly assigning the structure members is very helpful, especially in large +structure initializations, as this will help us to keep track of which structure +member has been initialized with what value. + +Use the sizeof operator when a structure has a 'size' field to be filled with the +structure size (sizeof(structureType);) + + +8) Accessing hardware registers: + +Hardware registers are stored in memory starting at 0xBC000000 and are _special_ +compared to global variables. They have to be written synchronously, which means +you have to tell the compiler that it can't invert two hardware operations. + +If you want to load from/store to a hardware register, use the HW(addr) macro. + +u32 hwRegTmp; + +hwRegTmp = HW(0xBC000000); +HW(0xBC000000) = 1; + + +9) Comments: + +All exported functions of a module and important data structures as well as +#defines shared among .c files have to be put into a header file used as a module documentation. + +Exported functions are commented like this: + +/** + * Purpose of . + * + * @param a Description of parameter a. + * @param b Description of parameter b. + * + * @return 0 on success. + */ +u32 exportedFunction(u32 a, u32 b); + +Note: "0 on success." implies that the function returns less than zero on error. +We do that because all the exported functions return less than zero on error. If +the function always returns 0, just use "@return 0.". + +Structure members are commented like this (same for enumerations): + +typedef struct { + /** The X-axis value of a point. */ + s32 x; + /** The Y-axis value of a point. */ + s32 y; +} Point; + +In the module's .c file(s), use "/*" instead of "/**" and only add +parameter desription if the purpose of the parameters is unclear without +additional description. + +/* + * Purpose of . + * + * Returns the sum of a and b. + */ +static s32 sum(s23 a, s32 b); + +Here, everything about the parameters a and b is obvious, so there is no need for +explaining them. +Also note that we use "Returns..." instead of "@return" for header functions. + +Comments inside of functions: + +When necessary, comment hard-to-understand code parts, explaining WHAT the code is doing. +The reader of the .c file probably knows a lot less than you (as you reversed the module) +about the module. Thus, make sure that those readers will understand what is going on. + +One line comments will look like this: + +/* My comment. */ + +Longer comments differ from short comments: + +/* + * My comment, which is too long + * to fit in one line. + */ + +Use "//" for temporary comments such as "//TODO: " or address comments like +sceKernelCpuResumeIntr(suspendFlag); //0x000020A8 + +Make sure to use enough address comments to be able to fix every line in in your +reversed code file file fastly and easily. Keep a local copy of your reversed code files +with included address comments and, optionally, TODO-comments, and upload a file version +WITHOUT address-, TODO-comments to the uOFW svn, as soon as your module fully works. + +10) Switches: + +Put a break at the end of each "case" and at the end of the "default". +Don't skip lines between the different labels ("case"s and "default"s). +In example: +switch (type) { +case 1: + do1(); + break; +case 2: + do2(); + break; +default: + doDefault(); + break; +} \ No newline at end of file