X-Git-Url: https://git.octo.it/?p=collectd.git;a=blobdiff_plain;f=docs%2Freview_comments.md;h=9bad458e980a8d55df0c44d9d8a644842378bae3;hp=bcb6fc0ce2ae86f6f8bf1ef83b9f85db9b923637;hb=33937e78d756ee11e59911e2ed25a74f4b6abdec;hpb=7946fba19c74ad4ed7a543ddeb0337d5dcf19359 diff --git a/docs/review_comments.md b/docs/review_comments.md index bcb6fc0c..9bad458e 100644 --- a/docs/review_comments.md +++ b/docs/review_comments.md @@ -16,8 +16,8 @@ A link to each paragraph is provided at the beginning for easy copy'n'pasting. → [https://collectd.org/review-comments#initialize-variables](https://collectd.org/review-comments#initialize-variables) Initialize variables when declaring them. By default, C does not initialize -variables when they are declared. If a code path ends up reading the variable -before it is initialized, for example because a loop body is never +local variables when they are defined. If a code path ends up reading the +variable before it is initialized, for example because a loop body is never executed, it will read random data, causing undefined behavior. Worst case, pointers will point to random memory causing a segmentation fault. @@ -47,21 +47,50 @@ compilers don't implement this correctly and will get confused when the first member is a struct or a union. Our *continuous integration* framework will catch these cases. -In many cases, this means declaring variables later. For example, this *bad* -example: +## Define variables on first use + +→ [https://collectd.org/review-comments#define-variables-on-first-use](https://collectd.org/review-comments#define-variables-on-first-use) + +Local variables should be defined when they are first used, ideally when they +can be initialized. For example: ```c -int status; /* BAD */ +struct foo *f = calloc(1, sizeof(*f)); +if (f == NULL) { + return ENOMEM; +} + +/* GOOD: status defiened and initialized late. */ +int status = function_call(f); +``` -/* other code */ +Sometimes variables are initialized by passing a pointer to them to a function. +In that case define them as close to the function call as you can and +zero-initialize them. The function may only partially initialize a struct or +not initialize a struct at all in some circumstances. -status = function_call(); +**Example:** + +```c +char const *path = determine_path(); + +struct stat s = {0}; +int status = stat(path, &s); ``` -Would become: +Old C standards (C89 and ealier) required variables to be defined at the +beginning of a scope block. The following *bad* style is still frequently +found: ```c -/* other code */ +/* BAD: local variables defined at beginning of block. */ +struct foo *f; +int status; + +f = calloc(1, sizeof(*f)); +if (f == NULL) { + return ENOMEM; +} -int status = function_call(); /* GOOD */ +status = function_call(f); ```