summaryrefslogtreecommitdiffstats
path: root/doc/coding-style.txt
diff options
context:
space:
mode:
authorDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-13 12:18:05 +0000
committerDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-13 12:18:05 +0000
commitb46aad6df449445a9fc4aa7b32bd40005438e3f7 (patch)
tree751aa858ca01f35de800164516b298887382919d /doc/coding-style.txt
parentInitial commit. (diff)
downloadhaproxy-b46aad6df449445a9fc4aa7b32bd40005438e3f7.tar.xz
haproxy-b46aad6df449445a9fc4aa7b32bd40005438e3f7.zip
Adding upstream version 2.9.5.upstream/2.9.5
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'doc/coding-style.txt')
-rw-r--r--doc/coding-style.txt1566
1 files changed, 1566 insertions, 0 deletions
diff --git a/doc/coding-style.txt b/doc/coding-style.txt
new file mode 100644
index 0000000..02a55f5
--- /dev/null
+++ b/doc/coding-style.txt
@@ -0,0 +1,1566 @@
+2020/07/07 - HAProxy coding style - Willy Tarreau <w@1wt.eu>
+------------------------------------------------------------
+
+A number of contributors are often embarrassed with coding style issues, they
+don't always know if they're doing it right, especially since the coding style
+has elvoved along the years. What is explained here is not necessarily what is
+applied in the code, but new code should as much as possible conform to this
+style. Coding style fixes happen when code is replaced. It is useless to send
+patches to fix coding style only, they will be rejected, unless they belong to
+a patch series which needs these fixes prior to get code changes. Also, please
+avoid fixing coding style in the same patches as functional changes, they make
+code review harder.
+
+A good way to quickly validate your patch before submitting it is to pass it
+through the Linux kernel's checkpatch.pl utility which can be downloaded here :
+
+ http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scripts/checkpatch.pl
+
+Running it with the following options relaxes its checks to accommodate to the
+extra degree of freedom that is tolerated in HAProxy's coding style compared to
+the stricter style used in the kernel :
+
+ checkpatch.pl -q --max-line-length=160 --no-tree --no-signoff \
+ --ignore=LEADING_SPACE,CODE_INDENT,DEEP_INDENTATION \
+ --ignore=ELSE_AFTER_BRACE < patch
+
+You can take its output as hints instead of strict rules, but in general its
+output will be accurate and it may even spot some real bugs.
+
+When modifying a file, you must accept the terms of the license of this file
+which is recalled at the top of the file, or is explained in the LICENSE file,
+or if not stated, defaults to LGPL version 2.1 or later for files in the
+'include' directory, and GPL version 2 or later for all other files.
+
+When adding a new file, you must add a copyright banner at the top of the
+file with your real name, e-mail address and a reminder of the license.
+Contributions under incompatible licenses or too restrictive licenses might
+get rejected. If in doubt, please apply the principle above for existing files.
+
+All code examples below will intentionally be prefixed with " | " to mark
+where the code aligns with the first column, and tabs in this document will be
+represented as a series of 8 spaces so that it displays the same everywhere.
+
+
+1) Indentation and alignment
+----------------------------
+
+1.1) Indentation
+----------------
+
+Indentation and alignment are two completely different things that people often
+get wrong. Indentation is used to mark a sub-level in the code. A sub-level
+means that a block is executed in the context of another block (eg: a function
+or a condition) :
+
+ | main(int argc, char **argv)
+ | {
+ | int i;
+ |
+ | if (argc < 2)
+ | exit(1);
+ | }
+
+In the example above, the code belongs to the main() function and the exit()
+call belongs to the if statement. Indentation is made with tabs (\t, ASCII 9),
+which allows any developer to configure their preferred editor to use their
+own tab size and to still get the text properly indented. Exactly one tab is
+used per sub-level. Tabs may only appear at the beginning of a line or after
+another tab. It is illegal to put a tab after some text, as it mangles displays
+in a different manner for different users (particularly when used to align
+comments or values after a #define). If you're tempted to put a tab after some
+text, then you're doing it wrong and you need alignment instead (see below).
+
+Note that there are places where the code was not properly indented in the
+past. In order to view it correctly, you may have to set your tab size to 8
+characters.
+
+
+1.2) Alignment
+--------------
+
+Alignment is used to continue a line in a way to makes things easier to group
+together. By definition, alignment is character-based, so it uses spaces. Tabs
+would not work because for one tab there would not be as many characters on all
+displays. For instance, the arguments in a function declaration may be broken
+into multiple lines using alignment spaces :
+
+ | int http_header_match2(const char *hdr, const char *end,
+ | const char *name, int len)
+ | {
+ | ...
+ | }
+
+In this example, the "const char *name" part is aligned with the first
+character of the group it belongs to (list of function arguments). Placing it
+here makes it obvious that it's one of the function's arguments. Multiple lines
+are easy to handle this way. This is very common with long conditions too :
+
+ | if ((len < eol - sol) &&
+ | (sol[len] == ':') &&
+ | (strncasecmp(sol, name, len) == 0)) {
+ | ctx->del = len;
+ | }
+
+If we take again the example above marking tabs with "[-Tabs-]" and spaces
+with "#", we get this :
+
+ | [-Tabs-]if ((len < eol - sol) &&
+ | [-Tabs-]####(sol[len] == ':') &&
+ | [-Tabs-]####(strncasecmp(sol, name, len) == 0)) {
+ | [-Tabs-][-Tabs-]ctx->del = len;
+ | [-Tabs-]}
+
+It is worth noting that some editors tend to confuse indentations and alignment.
+Emacs is notoriously known for this brokenness, and is responsible for almost
+all of the alignment mess. The reason is that Emacs only counts spaces, tries
+to fill as many as possible with tabs and completes with spaces. Once you know
+it, you just have to be careful, as alignment is not used much, so generally it
+is just a matter of replacing the last tab with 8 spaces when this happens.
+
+Indentation should be used everywhere there is a block or an opening brace. It
+is not possible to have two consecutive closing braces on the same column, it
+means that the innermost was not indented.
+
+Right :
+
+ | main(int argc, char **argv)
+ | {
+ | if (argc > 1) {
+ | printf("Hello\n");
+ | }
+ | exit(0);
+ | }
+
+Wrong :
+
+ | main(int argc, char **argv)
+ | {
+ | if (argc > 1) {
+ | printf("Hello\n");
+ | }
+ | exit(0);
+ | }
+
+A special case applies to switch/case statements. Due to my editor's settings,
+I've been used to align "case" with "switch" and to find it somewhat logical
+since each of the "case" statements opens a sublevel belonging to the "switch"
+statement. But indenting "case" after "switch" is accepted too. However in any
+case, whatever follows the "case" statement must be indented, whether or not it
+contains braces :
+
+ | switch (*arg) {
+ | case 'A': {
+ | int i;
+ | for (i = 0; i < 10; i++)
+ | printf("Please stop pressing 'A'!\n");
+ | break;
+ | }
+ | case 'B':
+ | printf("You pressed 'B'\n");
+ | break;
+ | case 'C':
+ | case 'D':
+ | printf("You pressed 'C' or 'D'\n");
+ | break;
+ | default:
+ | printf("I don't know what you pressed\n");
+ | }
+
+
+2) Braces
+---------
+
+Braces are used to delimit multiple-instruction blocks. In general it is
+preferred to avoid braces around single-instruction blocks as it reduces the
+number of lines :
+
+Right :
+
+ | if (argc >= 2)
+ | exit(0);
+
+Wrong :
+
+ | if (argc >= 2) {
+ | exit(0);
+ | }
+
+But it is not that strict, it really depends on the context. It happens from
+time to time that single-instruction blocks are enclosed within braces because
+it makes the code more symmetrical, or more readable. Example :
+
+ | if (argc < 2) {
+ | printf("Missing argument\n");
+ | exit(1);
+ | } else {
+ | exit(0);
+ | }
+
+Braces are always needed to declare a function. A function's opening brace must
+be placed at the beginning of the next line :
+
+Right :
+
+ | int main(int argc, char **argv)
+ | {
+ | exit(0);
+ | }
+
+Wrong :
+
+ | int main(int argc, char **argv) {
+ | exit(0);
+ | }
+
+Note that a large portion of the code still does not conforms to this rule, as
+it took years to get all authors to adapt to this more common standard which
+is now preferred, as it avoids visual confusion when function declarations are
+broken on multiple lines :
+
+Right :
+
+ | int foo(const char *hdr, const char *end,
+ | const char *name, const char *err,
+ | int len)
+ | {
+ | int i;
+
+Wrong :
+
+ | int foo(const char *hdr, const char *end,
+ | const char *name, const char *err,
+ | int len) {
+ | int i;
+
+Braces should always be used where there might be an ambiguity with the code
+later. The most common example is the stacked "if" statement where an "else"
+may be added later at the wrong place breaking the code, but it also happens
+with comments or long arguments in function calls. In general, if a block is
+more than one line long, it should use braces.
+
+Dangerous code waiting of a victim :
+
+ | if (argc < 2)
+ | /* ret must not be negative here */
+ | if (ret < 0)
+ | return -1;
+
+Wrong change :
+
+ | if (argc < 2)
+ | /* ret must not be negative here */
+ | if (ret < 0)
+ | return -1;
+ | else
+ | return 0;
+
+It will do this instead of what your eye seems to tell you :
+
+ | if (argc < 2)
+ | /* ret must not be negative here */
+ | if (ret < 0)
+ | return -1;
+ | else
+ | return 0;
+
+Right :
+
+ | if (argc < 2) {
+ | /* ret must not be negative here */
+ | if (ret < 0)
+ | return -1;
+ | }
+ | else
+ | return 0;
+
+Similarly dangerous example :
+
+ | if (ret < 0)
+ | /* ret must not be negative here */
+ | complain();
+ | init();
+
+Wrong change to silent the annoying message :
+
+ | if (ret < 0)
+ | /* ret must not be negative here */
+ | //complain();
+ | init();
+
+... which in fact means :
+
+ | if (ret < 0)
+ | init();
+
+
+3) Breaking lines
+-----------------
+
+There is no strict rule for line breaking. Some files try to stick to the 80
+column limit, but given that various people use various tab sizes, it does not
+make much sense. Also, code is sometimes easier to read with less lines, as it
+represents less surface on the screen (since each new line adds its tabs and
+spaces). The rule is to stick to the average line length of other lines. If you
+are working in a file which fits in 80 columns, try to keep this goal in mind.
+If you're in a function with 120-chars lines, there is no reason to add many
+short lines, so you can make longer lines.
+
+In general, opening a new block should lead to a new line. Similarly, multiple
+instructions should be avoided on the same line. But some constructs make it
+more readable when those are perfectly aligned :
+
+A copy-paste bug in the following construct will be easier to spot :
+
+ | if (omult % idiv == 0) { omult /= idiv; idiv = 1; }
+ | if (idiv % omult == 0) { idiv /= omult; omult = 1; }
+ | if (imult % odiv == 0) { imult /= odiv; odiv = 1; }
+ | if (odiv % imult == 0) { odiv /= imult; imult = 1; }
+
+than in this one :
+
+ | if (omult % idiv == 0) {
+ | omult /= idiv;
+ | idiv = 1;
+ | }
+ | if (idiv % omult == 0) {
+ | idiv /= omult;
+ | omult = 1;
+ | }
+ | if (imult % odiv == 0) {
+ | imult /= odiv;
+ | odiv = 1;
+ | }
+ | if (odiv % imult == 0) {
+ | odiv /= imult;
+ | imult = 1;
+ | }
+
+What is important is not to mix styles. For instance there is nothing wrong
+with having many one-line "case" statements as long as most of them are this
+short like below :
+
+ | switch (*arg) {
+ | case 'A': ret = 1; break;
+ | case 'B': ret = 2; break;
+ | case 'C': ret = 4; break;
+ | case 'D': ret = 8; break;
+ | default : ret = 0; break;
+ | }
+
+Otherwise, prefer to have the "case" statement on its own line as in the
+example in section 1.2 about alignment. In any case, avoid to stack multiple
+control statements on the same line, so that it will never be the needed to
+add two tab levels at once :
+
+Right :
+
+ | switch (*arg) {
+ | case 'A':
+ | if (ret < 0)
+ | ret = 1;
+ | break;
+ | default : ret = 0; break;
+ | }
+
+Wrong :
+
+ | switch (*arg) {
+ | case 'A': if (ret < 0)
+ | ret = 1;
+ | break;
+ | default : ret = 0; break;
+ | }
+
+Right :
+
+ | if (argc < 2)
+ | if (ret < 0)
+ | return -1;
+
+or Right :
+
+ | if (argc < 2)
+ | if (ret < 0) return -1;
+
+but Wrong :
+
+ | if (argc < 2) if (ret < 0) return -1;
+
+
+When complex conditions or expressions are broken into multiple lines, please
+do ensure that alignment is perfectly appropriate, and group all main operators
+on the same side (which you're free to choose as long as it does not change for
+every block. Putting binary operators on the right side is preferred as it does
+not mangle with alignment but various people have their preferences.
+
+Right :
+
+ | if ((txn->flags & TX_NOT_FIRST) &&
+ | ((req->flags & BF_FULL) ||
+ | req->r < req->lr ||
+ | req->r > req->data + req->size - global.tune.maxrewrite)) {
+ | return 0;
+ | }
+
+Right :
+
+ | if ((txn->flags & TX_NOT_FIRST)
+ | && ((req->flags & BF_FULL)
+ | || req->r < req->lr
+ | || req->r > req->data + req->size - global.tune.maxrewrite)) {
+ | return 0;
+ | }
+
+Wrong :
+
+ | if ((txn->flags & TX_NOT_FIRST) &&
+ | ((req->flags & BF_FULL) ||
+ | req->r < req->lr
+ | || req->r > req->data + req->size - global.tune.maxrewrite)) {
+ | return 0;
+ | }
+
+If it makes the result more readable, parenthesis may even be closed on their
+own line in order to align with the opening one. Note that should normally not
+be needed because such code would be too complex to be digged into.
+
+The "else" statement may either be merged with the closing "if" brace or lie on
+its own line. The later is preferred but it adds one extra line to each control
+block which is annoying in short ones. However, if the "else" is followed by an
+"if", then it should really be on its own line and the rest of the if/else
+blocks must follow the same style.
+
+Right :
+
+ | if (a < b) {
+ | return a;
+ | }
+ | else {
+ | return b;
+ | }
+
+Right :
+
+ | if (a < b) {
+ | return a;
+ | } else {
+ | return b;
+ | }
+
+Right :
+
+ | if (a < b) {
+ | return a;
+ | }
+ | else if (a != b) {
+ | return b;
+ | }
+ | else {
+ | return 0;
+ | }
+
+Wrong :
+
+ | if (a < b) {
+ | return a;
+ | } else if (a != b) {
+ | return b;
+ | } else {
+ | return 0;
+ | }
+
+Wrong :
+
+ | if (a < b) {
+ | return a;
+ | }
+ | else if (a != b) {
+ | return b;
+ | } else {
+ | return 0;
+ | }
+
+
+4) Spacing
+----------
+
+Correctly spacing code is very important. When you have to spot a bug at 3am,
+you need it to be clear. When you expect other people to review your code, you
+want it to be clear and don't want them to get nervous when trying to find what
+you did.
+
+Always place spaces around all binary or ternary operators, commas, as well as
+after semi-colons and opening braces if the line continues :
+
+Right :
+
+ | int ret = 0;
+ | /* if (x >> 4) { x >>= 4; ret += 4; } */
+ | ret += (x >> 4) ? (x >>= 4, 4) : 0;
+ | val = ret + ((0xFFFFAA50U >> (x << 1)) & 3) + 1;
+
+Wrong :
+
+ | int ret=0;
+ | /* if (x>>4) {x>>=4;ret+=4;} */
+ | ret+=(x>>4)?(x>>=4,4):0;
+ | val=ret+((0xFFFFAA50U>>(x<<1))&3)+1;
+
+Never place spaces after unary operators (&, *, -, !, ~, ++, --) nor cast, as
+they might be confused with they binary counterpart, nor before commas or
+semicolons :
+
+Right :
+
+ | bit = !!(~len++ ^ -(unsigned char)*x);
+
+Wrong :
+
+ | bit = ! ! (~len++ ^ - (unsigned char) * x) ;
+
+Note that "sizeof" is a unary operator which is sometimes considered as a
+language keyword, but in no case it is a function. It does not require
+parenthesis so it is sometimes followed by spaces and sometimes not when
+there are no parenthesis. Most people do not really care as long as what
+is written is unambiguous.
+
+Braces opening a block must be preceded by one space unless the brace is
+placed on the first column :
+
+Right :
+
+ | if (argc < 2) {
+ | }
+
+Wrong :
+
+ | if (argc < 2){
+ | }
+
+Do not add unneeded spaces inside parenthesis, they just make the code less
+readable.
+
+Right :
+
+ | if (x < 4 && (!y || !z))
+ | break;
+
+Wrong :
+
+ | if ( x < 4 && ( !y || !z ) )
+ | break;
+
+Language keywords must all be followed by a space. This is true for control
+statements (do, for, while, if, else, return, switch, case), and for types
+(int, char, unsigned). As an exception, the last type in a cast does not take
+a space before the closing parenthesis). The "default" statement in a "switch"
+construct is generally just followed by the colon. However the colon after a
+"case" or "default" statement must be followed by a space.
+
+Right :
+
+ | if (nbargs < 2) {
+ | printf("Missing arg at %c\n", *(char *)ptr);
+ | for (i = 0; i < 10; i++) beep();
+ | return 0;
+ | }
+ | switch (*arg) {
+
+Wrong :
+
+ | if(nbargs < 2){
+ | printf("Missing arg at %c\n", *(char*)ptr);
+ | for(i = 0; i < 10; i++)beep();
+ | return 0;
+ | }
+ | switch(*arg) {
+
+Function calls are different, the opening parenthesis is always coupled to the
+function name without any space. But spaces are still needed after commas :
+
+Right :
+
+ | if (!init(argc, argv))
+ | exit(1);
+
+Wrong :
+
+ | if (!init (argc,argv))
+ | exit(1);
+
+
+5) Excess or lack of parenthesis
+--------------------------------
+
+Sometimes there are too many parenthesis in some formulas, sometimes there are
+too few. There are a few rules of thumb for this. The first one is to respect
+the compiler's advice. If it emits a warning and asks for more parenthesis to
+avoid confusion, follow the advice at least to shut the warning. For instance,
+the code below is quite ambiguous due to its alignment :
+
+ | if (var1 < 2 || var2 < 2 &&
+ | var3 != var4) {
+ | /* fail */
+ | return -3;
+ | }
+
+Note that this code does :
+
+ | if (var1 < 2 || (var2 < 2 && var3 != var4)) {
+ | /* fail */
+ | return -3;
+ | }
+
+But maybe the author meant :
+
+ | if ((var1 < 2 || var2 < 2) && var3 != var4) {
+ | /* fail */
+ | return -3;
+ | }
+
+A second rule to put parenthesis is that people don't always know operators
+precedence too well. Most often they have no issue with operators of the same
+category (eg: booleans, integers, bit manipulation, assignment) but once these
+operators are mixed, it causes them all sort of issues. In this case, it is
+wise to use parenthesis to avoid errors. One common error concerns the bit
+shift operators because they're used to replace multiplies and divides but
+don't have the same precedence :
+
+The expression :
+
+ | x = y * 16 + 5;
+
+becomes :
+
+ | x = y << 4 + 5;
+
+which is wrong because it is equivalent to :
+
+ | x = y << (4 + 5);
+
+while the following was desired instead :
+
+ | x = (y << 4) + 5;
+
+It is generally fine to write boolean expressions based on comparisons without
+any parenthesis. But on top of that, integer expressions and assignments should
+then be protected. For instance, there is an error in the expression below
+which should be safely rewritten :
+
+Wrong :
+
+ | if (var1 > 2 && var1 < 10 ||
+ | var1 > 2 + 256 && var2 < 10 + 256 ||
+ | var1 > 2 + 1 << 16 && var2 < 10 + 2 << 16)
+ | return 1;
+
+Right (may remove a few parenthesis depending on taste) :
+
+ | if ((var1 > 2 && var1 < 10) ||
+ | (var1 > (2 + 256) && var2 < (10 + 256)) ||
+ | (var1 > (2 + (1 << 16)) && var2 < (10 + (1 << 16))))
+ | return 1;
+
+The "return" statement is not a function, so it takes no argument. It is a
+control statement which is followed by the expression to be returned. It does
+not need to be followed by parenthesis :
+
+Wrong :
+
+ | int ret0()
+ | {
+ | return(0);
+ | }
+
+Right :
+
+ | int ret0()
+ | {
+ | return 0;
+ | }
+
+Parenthesisis are also found in type casts. Type casting should be avoided as
+much as possible, especially when it concerns pointer types. Casting a pointer
+disables the compiler's type checking and is the best way to get caught doing
+wrong things with data not the size you expect. If you need to manipulate
+multiple data types, you can use a union instead. If the union is really not
+convenient and casts are easier, then try to isolate them as much as possible,
+for instance when initializing function arguments or in another function. Not
+proceeding this way causes huge risks of not using the proper pointer without
+any notification, which is especially true during copy-pastes.
+
+Wrong :
+
+ | void *check_private_data(void *arg1, void *arg2)
+ | {
+ | char *area;
+ |
+ | if (*(int *)arg1 > 1000)
+ | return NULL;
+ | if (memcmp(*(const char *)arg2, "send(", 5) != 0))
+ | return NULL;
+ | area = malloc(*(int *)arg1);
+ | if (!area)
+ | return NULL;
+ | memcpy(area, *(const char *)arg2 + 5, *(int *)arg1);
+ | return area;
+ | }
+
+Right :
+
+ | void *check_private_data(void *arg1, void *arg2)
+ | {
+ | char *area;
+ | int len = *(int *)arg1;
+ | const char *msg = arg2;
+ |
+ | if (len > 1000)
+ | return NULL;
+ | if (memcmp(msg, "send(", 5) != 0)
+ | return NULL;
+ | area = malloc(len);
+ | if (!area)
+ | return NULL;
+ | memcpy(area, msg + 5, len);
+ | return area;
+ | }
+
+
+6) Ambiguous comparisons with zero or NULL
+------------------------------------------
+
+In C, '0' has no type, or it has the type of the variable it is assigned to.
+Comparing a variable or a return value with zero means comparing with the
+representation of zero for this variable's type. For a boolean, zero is false.
+For a pointer, zero is NULL. Very often, to make things shorter, it is fine to
+use the '!' unary operator to compare with zero, as it is shorter and easier to
+remind or understand than a plain '0'. Since the '!' operator is read "not", it
+helps read code faster when what follows it makes sense as a boolean, and it is
+often much more appropriate than a comparison with zero which makes an equal
+sign appear at an undesirable place. For instance :
+
+ | if (!isdigit(*c) && !isspace(*c))
+ | break;
+
+is easier to understand than :
+
+ | if (isdigit(*c) == 0 && isspace(*c) == 0)
+ | break;
+
+For a char this "not" operator can be reminded as "no remaining char", and the
+absence of comparison to zero implies existence of the tested entity, hence the
+simple strcpy() implementation below which automatically stops once the last
+zero is copied :
+
+ | void my_strcpy(char *d, const char *s)
+ | {
+ | while ((*d++ = *s++));
+ | }
+
+Note the double parenthesis in order to avoid the compiler telling us it looks
+like an equality test.
+
+For a string or more generally any pointer, this test may be understood as an
+existence test or a validity test, as the only pointer which will fail to
+validate equality is the NULL pointer :
+
+ | area = malloc(1000);
+ | if (!area)
+ | return -1;
+
+However sometimes it can fool the reader. For instance, strcmp() precisely is
+one of such functions whose return value can make one think the opposite due to
+its name which may be understood as "if strings compare...". Thus it is strongly
+recommended to perform an explicit comparison with zero in such a case, and it
+makes sense considering that the comparison's operator is the same that is
+wanted to compare the strings (note that current config parser lacks a lot in
+this regards) :
+
+ strcmp(a, b) == 0 <=> a == b
+ strcmp(a, b) != 0 <=> a != b
+ strcmp(a, b) < 0 <=> a < b
+ strcmp(a, b) > 0 <=> a > b
+
+Avoid this :
+
+ | if (strcmp(arg, "test"))
+ | printf("this is not a test\n");
+ |
+ | if (!strcmp(arg, "test"))
+ | printf("this is a test\n");
+
+Prefer this :
+
+ | if (strcmp(arg, "test") != 0)
+ | printf("this is not a test\n");
+ |
+ | if (strcmp(arg, "test") == 0)
+ | printf("this is a test\n");
+
+
+7) System call returns
+----------------------
+
+This is not directly a matter of coding style but more of bad habits. It is
+important to check for the correct value upon return of syscalls. The proper
+return code indicating an error is described in its man page. There is no
+reason to consider wider ranges than what is indicated. For instance, it is
+common to see such a thing :
+
+ | if ((fd = open(file, O_RDONLY)) < 0)
+ | return -1;
+
+This is wrong. The man page says that -1 is returned if an error occurred. It
+does not suggest that any other negative value will be an error. It is possible
+that a few such issues have been left in existing code. They are bugs for which
+fixes are accepted, even though they're currently harmless since open() is not
+known for returning negative values at the moment.
+
+
+8) Declaring new types, names and values
+----------------------------------------
+
+Please refrain from using "typedef" to declare new types, they only obfuscate
+the code. The reader never knows whether he's manipulating a scalar type or a
+struct. For instance it is not obvious why the following code fails to build :
+
+ | int delay_expired(timer_t exp, timer_us_t now)
+ | {
+ | return now >= exp;
+ | }
+
+With the types declared in another file this way :
+
+ | typedef unsigned int timer_t;
+ | typedef struct timeval timer_us_t;
+
+This cannot work because we're comparing a scalar with a struct, which does
+not make sense. Without a typedef, the function would have been written this
+way without any ambiguity and would not have failed :
+
+ | int delay_expired(unsigned int exp, struct timeval *now)
+ | {
+ | return now >= exp->tv_sec;
+ | }
+
+Declaring special values may be done using enums. Enums are a way to define
+structured integer values which are related to each other. They are perfectly
+suited for state machines. While the first element is always assigned the zero
+value, not everybody knows that, especially people working with multiple
+languages all the day. For this reason it is recommended to explicitly force
+the first value even if it's zero. The last element should be followed by a
+comma if it is planned that new elements might later be added, this will make
+later patches shorter. Conversely, if the last element is placed in order to
+get the number of possible values, it must not be followed by a comma and must
+be preceded by a comment :
+
+ | enum {
+ | first = 0,
+ | second,
+ | third,
+ | fourth,
+ | };
+
+
+ | enum {
+ | first = 0,
+ | second,
+ | third,
+ | fourth,
+ | /* nbvalues must always be placed last */
+ | nbvalues
+ | };
+
+Structure names should be short enough not to mangle function declarations,
+and explicit enough to avoid confusion (which is the most important thing).
+
+Wrong :
+
+ | struct request_args { /* arguments on the query string */
+ | char *name;
+ | char *value;
+ | struct misc_args *next;
+ | };
+
+Right :
+
+ | struct qs_args { /* arguments on the query string */
+ | char *name;
+ | char *value;
+ | struct qs_args *next;
+ | }
+
+
+When declaring new functions or structures, please do not use CamelCase, which
+is a style where upper and lower case are mixed in a single word. It causes a
+lot of confusion when words are composed from acronyms, because it's hard to
+stick to a rule. For instance, a function designed to generate an ISN (initial
+sequence number) for a TCP/IP connection could be called :
+
+ - generateTcpipIsn()
+ - generateTcpIpIsn()
+ - generateTcpIpISN()
+ - generateTCPIPISN()
+ etc...
+
+None is right, none is wrong, these are just preferences which might change
+along the code. Instead, please use an underscore to separate words. Lowercase
+is preferred for the words, but if acronyms are upcased it's not dramatic. The
+real advantage of this method is that it creates unambiguous levels even for
+short names.
+
+Valid examples :
+
+ - generate_tcpip_isn()
+ - generate_tcp_ip_isn()
+ - generate_TCPIP_ISN()
+ - generate_TCP_IP_ISN()
+
+Another example is easy to understand when 3 arguments are involved in naming
+the function :
+
+Wrong (naming conflict) :
+
+ | /* returns A + B * C */
+ | int mulABC(int a, int b, int c)
+ | {
+ | return a + b * c;
+ | }
+ |
+ | /* returns (A + B) * C */
+ | int mulABC(int a, int b, int c)
+ | {
+ | return (a + b) * c;
+ | }
+
+Right (unambiguous naming) :
+
+ | /* returns A + B * C */
+ | int mul_a_bc(int a, int b, int c)
+ | {
+ | return a + b * c;
+ | }
+ |
+ | /* returns (A + B) * C */
+ | int mul_ab_c(int a, int b, int c)
+ | {
+ | return (a + b) * c;
+ | }
+
+Whenever you manipulate pointers, try to declare them as "const", as it will
+save you from many accidental misuses and will only cause warnings to be
+emitted when there is a real risk. In the examples below, it is possible to
+call my_strcpy() with a const string only in the first declaration. Note that
+people who ignore "const" are often the ones who cast a lot and who complain
+from segfaults when using strtok() !
+
+Right :
+
+ | void my_strcpy(char *d, const char *s)
+ | {
+ | while ((*d++ = *s++));
+ | }
+ |
+ | void say_hello(char *dest)
+ | {
+ | my_strcpy(dest, "hello\n");
+ | }
+
+Wrong :
+
+ | void my_strcpy(char *d, char *s)
+ | {
+ | while ((*d++ = *s++));
+ | }
+ |
+ | void say_hello(char *dest)
+ | {
+ | my_strcpy(dest, "hello\n");
+ | }
+
+
+9) Getting macros right
+-----------------------
+
+It is very common for macros to do the wrong thing when used in a way their
+author did not have in mind. For this reason, macros must always be named with
+uppercase letters only. This is the only way to catch the developer's eye when
+using them, so that they double-check whether they are taking a risk or not. First,
+macros must never ever be terminated by a semi-colon, or they will close the
+wrong block once in a while. For instance, the following will cause a build
+error before the "else" due to the double semi-colon :
+
+Wrong :
+
+ | #define WARN printf("warning\n");
+ | ...
+ | if (a < 0)
+ | WARN;
+ | else
+ | a--;
+
+Right :
+
+ | #define WARN printf("warning\n")
+
+If multiple instructions are needed, then use a do { } while (0) block, which
+is the only construct which respects *exactly* the semantics of a single
+instruction :
+
+ | #define WARN do { printf("warning\n"); log("warning\n"); } while (0)
+ | ...
+ |
+ | if (a < 0)
+ | WARN;
+ | else
+ | a--;
+
+Second, do not put unprotected control statements in macros, they will
+definitely cause bugs :
+
+Wrong :
+
+ | #define WARN if (verbose) printf("warning\n")
+ | ...
+ | if (a < 0)
+ | WARN;
+ | else
+ | a--;
+
+Which is equivalent to the undesired form below :
+
+ | if (a < 0)
+ | if (verbose)
+ | printf("warning\n");
+ | else
+ | a--;
+
+Right way to do it :
+
+ | #define WARN do { if (verbose) printf("warning\n"); } while (0)
+ | ...
+ | if (a < 0)
+ | WARN;
+ | else
+ | a--;
+
+Which is equivalent to :
+
+ | if (a < 0)
+ | do { if (verbose) printf("warning\n"); } while (0);
+ | else
+ | a--;
+
+Macro parameters must always be surrounded by parenthesis, and must never be
+duplicated in the same macro unless explicitly stated. Also, macros must not be
+defined with operators without surrounding parenthesis. The MIN/MAX macros are
+a pretty common example of multiple misuses, but this happens as early as when
+using bit masks. Most often, in case of any doubt, try to use inline functions
+instead.
+
+Wrong :
+
+ | #define MIN(a, b) a < b ? a : b
+ |
+ | /* returns 2 * min(a,b) + 1 */
+ | int double_min_p1(int a, int b)
+ | {
+ | return 2 * MIN(a, b) + 1;
+ | }
+
+What this will do :
+
+ | int double_min_p1(int a, int b)
+ | {
+ | return 2 * a < b ? a : b + 1;
+ | }
+
+Which is equivalent to :
+
+ | int double_min_p1(int a, int b)
+ | {
+ | return (2 * a) < b ? a : (b + 1);
+ | }
+
+The first thing to fix is to surround the macro definition with parenthesis to
+avoid this mistake :
+
+ | #define MIN(a, b) (a < b ? a : b)
+
+But this is still not enough, as can be seen in this example :
+
+ | /* compares either a or b with c */
+ | int min_ab_c(int a, int b, int c)
+ | {
+ | return MIN(a ? a : b, c);
+ | }
+
+Which is equivalent to :
+
+ | int min_ab_c(int a, int b, int c)
+ | {
+ | return (a ? a : b < c ? a ? a : b : c);
+ | }
+
+Which in turn means a totally different thing due to precedence :
+
+ | int min_ab_c(int a, int b, int c)
+ | {
+ | return (a ? a : ((b < c) ? (a ? a : b) : c));
+ | }
+
+This can be fixed by surrounding *each* argument in the macro with parenthesis:
+
+ | #define MIN(a, b) ((a) < (b) ? (a) : (b))
+
+But this is still not enough, as can be seen in this example :
+
+ | int min_ap1_b(int a, int b)
+ | {
+ | return MIN(++a, b);
+ | }
+
+Which is equivalent to :
+
+ | int min_ap1_b(int a, int b)
+ | {
+ | return ((++a) < (b) ? (++a) : (b));
+ | }
+
+Again, this is wrong because "a" is incremented twice if below b. The only way
+to fix this is to use a compound statement and to assign each argument exactly
+once to a local variable of the same type :
+
+ | #define MIN(a, b) ({ typeof(a) __a = (a); typeof(b) __b = (b); \
+ | ((__a) < (__b) ? (__a) : (__b)); \
+ | })
+
+At this point, using static inline functions is much cleaner if a single type
+is to be used :
+
+ | static inline int min(int a, int b)
+ | {
+ | return a < b ? a : b;
+ | }
+
+
+10) Includes
+------------
+
+Includes are as much as possible listed in alphabetically ordered groups :
+ - the includes more or less system-specific (sys/*, netinet/*, ...)
+ - the libc-standard includes (those without any path component)
+ - includes from the local "import" subdirectory
+ - includes from the local "haproxy" subdirectory
+
+Each section is just visually delimited from the other ones using an empty
+line. The two first ones above may be merged into a single section depending on
+developer's preference. Please do not copy-paste include statements from other
+files. Having too many includes significantly increases build time and makes it
+hard to find which ones are needed later. Just include what you need and if
+possible in alphabetical order so that when something is missing, it becomes
+obvious where to look for it and where to add it.
+
+All files should include <haproxy/api.h> because this is where build options
+are prepared.
+
+HAProxy header files are split in two, those exporting the types only (named
+with a trailing "-t") and those exporting variables, functions and inline
+functions. Types, structures, enums and #defines must go into the types files
+which are the only ones that may be included by othertype files. Function
+prototypes and inlined functions must go into the main files. This split is
+because of inlined functions which cross-reference types from other files,
+which cause a chicken-and-egg problem if the functions and types are declared
+at the same place.
+
+Include files must be protected against multiple inclusion using the common
+#ifndef/#define/#endif trick with a tag derived from the include file and its
+location.
+
+
+11) Comments
+------------
+
+Comments are preferably of the standard 'C' form using /* */. The C++ form "//"
+are tolerated for very short comments (eg: a word or two) but should be avoided
+as much as possible. Multi-line comments are made with each intermediate line
+starting with a star aligned with the first one, as in this example :
+
+ | /*
+ | * This is a multi-line
+ | * comment.
+ | */
+
+If multiple code lines need a short comment, try to align them so that you can
+have multi-line sentences. This is rarely needed, only for really complex
+constructs.
+
+Do not tell what you're doing in comments, but explain why you're doing it if
+it seems not to be obvious. Also *do* indicate at the top of function what they
+accept and what they don't accept. For instance, strcpy() only accepts output
+buffers at least as large as the input buffer, and does not support any NULL
+pointer. There is nothing wrong with that if the caller knows it.
+
+Wrong use of comments :
+
+ | int flsnz8(unsigned int x)
+ | {
+ | int ret = 0; /* initialize ret */
+ | if (x >> 4) { x >>= 4; ret += 4; } /* add 4 to ret if needed */
+ | return ret + ((0xFFFFAA50U >> (x << 1)) & 3) + 1; /* add ??? */
+ | }
+ | ...
+ | bit = ~len + (skip << 3) + 9; /* update bit */
+
+Right use of comments :
+
+ | /* This function returns the position of the highest bit set in the lowest
+ | * byte of <x>, between 0 and 7. It only works if <x> is non-null. It uses
+ | * a 32-bit value as a lookup table to return one of 4 values for the
+ | * highest 16 possible 4-bit values.
+ | */
+ | int flsnz8(unsigned int x)
+ | {
+ | int ret = 0;
+ | if (x >> 4) { x >>= 4; ret += 4; }
+ | return ret + ((0xFFFFAA50U >> (x << 1)) & 3) + 1;
+ | }
+ | ...
+ | bit = ~len + (skip << 3) + 9; /* (skip << 3) + (8 - len), saves 1 cycle */
+
+
+12) Use of assembly
+-------------------
+
+There are many projects where use of assembly code is not welcome. There is no
+problem with use of assembly in haproxy, provided that :
+
+ a) an alternate C-form is provided for architectures not covered
+ b) the code is small enough and well commented enough to be maintained
+
+It is important to take care of various incompatibilities between compiler
+versions, for instance regarding output and cloberred registers. There are
+a number of documentations on the subject on the net. Anyway if you are
+fiddling with assembly, you probably know that already.
+
+Example :
+ | /* gcc does not know when it can safely divide 64 bits by 32 bits. Use this
+ | * function when you know for sure that the result fits in 32 bits, because
+ | * it is optimal on x86 and on 64bit processors.
+ | */
+ | static inline unsigned int div64_32(unsigned long long o1, unsigned int o2)
+ | {
+ | unsigned int result;
+ | #ifdef __i386__
+ | asm("divl %2"
+ | : "=a" (result)
+ | : "A"(o1), "rm"(o2));
+ | #else
+ | result = o1 / o2;
+ | #endif
+ | return result;
+ | }
+
+
+13) Pointers
+------------
+
+A lot could be said about pointers, there's enough to fill entire books. Misuse
+of pointers is one of the primary reasons for bugs in haproxy, and this rate
+has significantly increased with the use of threads. Moreover, bogus pointers
+cause the hardest to analyse bugs, because usually they result in modifications
+to reassigned areas or accesses to unmapped areas, and in each case, bugs that
+strike very far away from where they were located. Some bugs have already taken
+up to 3 weeks of full time analysis, which has a severe impact on the project's
+ability to make forward progress on important features. For this reason, code
+that doesn't look robust enough or that doesn't follow some of the rules below
+will be rejected, and may even be reverted after being merged if the trouble is
+detected late!
+
+
+13.1) No test before freeing
+----------------------------
+
+All platforms where haproxy is supported have a well-defined and documented
+behavior for free(NULL), which is to do nothing at all. In other words, free()
+does test for the pointer's nullity. As such, there is no point in testing
+if a pointer is NULL or not before calling free(). And further, you must not
+do it, because it adds some confusion to the reader during debugging sessions,
+making one think that the code's authors weren't very sure about what they
+were doing. This will not cause a bug but will result in your code to get
+rejected.
+
+Wrong call to free :
+
+ | static inline int blah_free(struct blah *blah)
+ | {
+ | if (blah->str1)
+ | free(blah->str1);
+ | if (blah->str2)
+ | free(blah->str2);
+ | free(blah);
+ | }
+
+Correct call to free :
+
+ | static inline int blah_free(struct blah *blah)
+ | {
+ | free(blah->str1);
+ | free(blah->str2);
+ | free(blah);
+ | }
+
+
+13.2) No dangling pointers
+--------------------------
+
+Pointers are very commonly used as booleans: if they're not NULL, then the
+area they point to is valid and may be used. This is convenient for many things
+and is even emphasized with threads where they can atomically be swapped with
+another value (even NULL), and as such provide guaranteed atomic resource
+allocation and sharing.
+
+The problem with this is when someone forgets to delete a pointer when an area
+is no longer valid, because this may result in the pointer being accessed later
+and pointing to a wrong location, one that was reallocated for something else
+and causing all sort of nastiness like crashes or memory corruption. Moreover,
+thanks to the memory pools, it is extremely likely that a just released pointer
+will be reassigned to a similar object with comparable values (flags etc) at
+the same positions, making tests apparently succeed for a while. Some such bugs
+have gone undetected for several years.
+
+The rule is pretty simple:
+
+ +-----------------------------------------------------------------+
+ | NO REACHABLE POINTER MAY EVER POINT TO AN UNREACHABLE LOCATION. |
+ +-----------------------------------------------------------------+
+
+By "reachable pointer", here we mean a pointer that is accessible from a
+reachable structure or a global variable. This means that any pointer found
+anywhere in any structure in the code may always be dereferenced. This can
+seem obvious but this is not always enforced.
+
+This means that when freeing an area, the pointer that was used to find that
+area must be overwritten with NULL, and all other such pointers must as well
+if any. It is one case where one can find more convenient to write the NULL
+on the same line as the call to free() to make things easier to check. Be
+careful about any potential "if" when doing this.
+
+Wrong use of free :
+
+ | static inline int blah_recycle(struct blah *blah)
+ | {
+ | free(blah->str1);
+ | free(blah->str2);
+ | }
+
+Correct use of free :
+
+ | static inline int blah_recycle(struct blah *blah)
+ | {
+ | free(blah->str1); blah->str1 = NULL;
+ | free(blah->str2); blah->str2 = NULL;
+ | }
+
+Sometimes the code doesn't permit this to be done. It is not a matter of code
+but a matter of architecture. Example:
+
+Initialization:
+
+ | static struct foo *foo_init()
+ | {
+ | struct foo *foo;
+ | struct bar *bar;
+ |
+ | foo = pool_alloc(foo_head);
+ | bar = pool_alloc(bar_head);
+ | if (!foo || !bar)
+ | goto fail;
+ | foo->bar = bar;
+ | ...
+ | }
+
+Scheduled task 1:
+
+ | static inline int foo_timeout(struct foo *foo)
+ | {
+ | free(foo->bar);
+ | free(foo);
+ | }
+
+Scheduled task 2:
+
+ | static inline int bar_timeout(struct bar *bar)
+ | {
+ | free(bar);
+ | }
+
+Here it's obvious that if "bar" times out, it will be freed but its pointer in
+"foo" will remain here, and if foo times out just after, it will lead to a
+double free. Or worse, if another instance allocates a pointer and receives bar
+again, when foo times out, it will release the old bar pointer which now points
+to a new object, and the code using that new object will crash much later, or
+even worse, will share the same area as yet another instance having inherited
+that pointer again.
+
+Here this simply means that the data model is wrong. If bar may be freed alone,
+it MUST have a pointer to foo so that bar->foo->bar is set to NULL to let foo
+finish its life peacefully. This also means that the code dealing with foo must
+be written in a way to support bar's leaving.
+
+
+13.3) Don't abuse pointers as booleans
+--------------------------------------
+
+Given the common use of a pointer to know if the area it points to is valid,
+there is a big incentive in using such pointers as booleans to describe
+something a bit higher level, like "is the user authenticated". This must not
+be done. The reason stems from the points above. Initially this perfectly
+matches and the code is simple. Then later some extra options need to be added,
+and more pointers are needed, all allocated together. At this point they all
+start to become their own booleans, supposedly always equivalent, but if that
+were true, they would be a single area with a single pointer. And things start
+to fall apart with some code areas relying on one pointer for the condition and
+other ones relying on other pointers. Pointers may be substituted with "flags"
+or "present in list" etc here. And from this point, things quickly degrade with
+pointers needing to remain set even if pointing to wrong areas, just for the
+sake of not being NULL and not breaking some assumptions. At this point the
+bugs are already there and the code is not trustable anymore.
+
+The only way to avoid this is to strictly respect this rule: pointers do not
+represent a functionality but a storage area. Of course it is very frequent to
+consider that if an optional string is not set, a feature is not enabled. This
+can be fine to some extents. But as soon as any slightest condition is added
+anywhere into the mux, the code relying on the pointer must be replaced with
+something else so that the pointer may live its own life and be released (and
+reset) earlier if needed.
+
+
+13.4) Mixing const and non-const
+--------------------------------
+
+Something often encountered, especially when assembling error messages, is
+functions that collect strings, assemble them into larger messages and free
+everything. The problem here is that if strings are defined as variables, there
+will rightfully be build warnings when reporting string constants such as bare
+keywords or messages, and if strings are defined as constants, it is not
+possible to free them. The temptation is sometimes huge to force some free()
+calls on casted strings. Do not do that! It will inevitably lead to someone
+getting caught passing a constant string that will make the process crash (if
+lucky). Document the expectations, indicate that all arguments must be freeable
+and that the caller must be capable of strdup(), and make your function support
+NULLs and document it (so that callers can deal with a failing strdup() on
+allocation error).
+
+One valid alternative is to use a secondary channel to indicate whether the
+message may be freed or not. A flag in a complex structure can be used for this
+purpose, for example. If you are certain that your strings are aligned to a
+certain number of bytes, it can be possible to instrument the code to use the
+lowest bit to indicate the need to free (e.g. by always adding one to every
+const string). But such a solution will require good enough instrumentation so
+that it doesn't constitute a new set of traps.
+
+
+13.5) No pointer casts
+----------------------
+
+Except in rare occasions caused by legacy APIs (e.g. sockaddr) or special cases
+which explicitly require a form of aliasing, there is no valid reason for
+casting pointers, and usually this is used to hide other problems that will
+strike later. The only suitable type of cast is the cast from the generic void*
+used to store a context for example. But in C, there is no need to cast to nor
+from void*, so this is not required. However those coming from C++ tend to be
+used to this practice, and others argue that it makes the intent more visible.
+
+As a corollary, do not abuse void*. Placing void* everywhere to avoid casting
+is a bad practice as well. The use of void* is only for generic functions or
+structures which do not have a limited set of types supported. When only a few
+types are supported, generally their type can be passed using a side channel,
+and the void* can be turned into a union that makes the code more readable and
+more verifiable.
+
+An alternative in haproxy is to use a pointer to an obj_type enum. Usually it
+is placed at the beginning of a structure. It works like a void* except that
+the type is read directly from the object. This is convenient when a small set
+of remote objects may be attached to another one because a single of them will
+match a non-null pointer (e.g. a connection or an applet).
+
+Example:
+
+ | static inline int blah_free(struct blah *blah)
+ | {
+ | /* only one of them (at most) will not be null */
+ | pool_free(pool_head_connection, objt_conn(blah->target));
+ | pool_free(pool_head_appctx, objt_appctx(blah->target));
+ | pool_free(pool_head_stream, objt_stream(blah->target));
+ | blah->target = NULL;
+ | }
+
+
+13.6) Extreme caution when using non-canonical pointers
+-------------------------------------------------------
+
+It can be particularly convenient to embed some logic in the unused bits or
+code points of a pointer. Indeed, when it is known by design that a given
+pointer will always follow a certain alignment, a few lower bits will always
+remain zero, and as such may be used as optional flags. For example, the ebtree
+code uses the lowest bit to differentiate left/right attachments to the parent
+and node/leaf in branches. It is also known that values very close to NULL will
+never represent a valid pointer, and the thread-safe MT_LIST code uses this to
+lock visited pointers.
+
+There are a few rules to respect in order to do this:
+ - the deviations from the canonical pointers must be exhaustively documented
+ where the pointer type is defined, and the whole control logic with its
+ implications and possible and impossible cases must be enumerated as well ;
+
+ - make sure that the operations will work on every supported platform, which
+ includes 32-bit platforms where structures may be aligned on as little as
+ 32-bit. 32-bit alignment leaves only two LSB available. When doing so, make
+ sure the target structures are not labelled with the "packed" attribute, or
+ that they're always perfectly aligned. All platforms where haproxy runs
+ have their NULL pointer mapped at address zero, and use page sizes at least
+ 4096 bytes large, leaving all values form 1 to 4095 unused. Anything
+ outside of this is unsafe. In particular, never use negative numbers to
+ represent a supposedly invalid address. On 32-bits platforms it will often
+ correspond to a system address or a special page. Always try a variety of
+ platforms when doing such a thing.
+
+ - the code must not use such pointers as booleans anymore even if it is known
+ that "it works" because that keeps a doubt open for the reviewer. Only the
+ canonical pointer may be tested. There can be a rare exception which is if
+ this is on a critical path where severe performance degradation may result
+ from this. In this case, *each* of the checks must be duly documented and
+ the equivalent BUG_ON() instances must be placed to prove the claim.
+
+ - some inline functions (or macros) must be used to turn the pointers to/from
+ their canonical form so that the regular code doesn't have to see the
+ operations, and so that the representation may be easily adjusted in the
+ future. A few comments indicating to a human how to turn a pointer back and
+ forth from inside a debugger will be appreciated, as macros often end up
+ not being trivially readable nor directly usable.
+
+ - do not use int types to cast the pointers, this will only work on 32-bit
+ platforms. While "long" is usually fine, it is not recommended anymore due
+ to the Windows platform being LLP64 and having it set to 32 bits. And
+ "long long" isn't good either for always being 64 bits. More suitable types
+ are ptrdiff_t or size_t. Note that while those were not available everywhere
+ in the early days of hparoxy, size_t is now heavily used and known to work
+ everywhere. And do not perform the operations on the pointers, only on the
+ integer types (and cast back again). Some compilers such as gcc are
+ extremely picky about this and will often emit wrong code when they see
+ equality conditions they believe is impossible and decide to optimize them
+ away.
+
+
+13.7) Pointers in unions
+------------------------
+
+Before placing multiple aliasing pointers inside a same union, there MUST be a
+SINGLE well-defined way to figure them out from each other. It may be thanks to
+a side-channel information (as done in the samples with a defined type), it may
+be based on in-area information (as done using obj_types), or any other trusted
+solution. In any case, if pointers are mixed with any other type (integer or
+float) in a union, there must be a very simple way to distinguish them, and not
+a platform-dependent nor compiler-dependent one.