Merge Christopher's changes #2
Conversation
…tyle of the document and match the style used in K&R's book. Braces are placed at the start of a line when they enclose a function definition. Asterisks are placed such that they accurately reflect the grammar of the language (in which * is part of a declarator, not part of the declaration-specifiers). Fixed errors and style of example 157: _file instead of *file, use of 0 as a null pointer constant, use of undefined macro IOFBF and a redundant check for buf == NULL. Substituted a read of the buffer for a write, because that seems like a more straightforward example of using the contents of the array.
Fixed another instance of misuse of _ in place of *.
|
Chris, I've reviewed your commits. |
|
In your first commit you modified the indentation to match the K&R book style. As you know there are m any indentation styles and the one thing everyone agrees on is that we should pick one indentation style and use it consistently. The "Examples of UB" document currently uses the "1 true brace style" (1TBS) as described here: You are suggesting we use "K&R style" instead. This is similar to 1TBS except when it comes to function definitions. I will note that several text editors prefer K&R style, as it is easy to identify a line with an open brace as a function definition beginning. Why should we use K&R style rather than 1TBS? |
|
WRT casting the result of malloc(), the current examples comply with CERT guideline MEM02-C:
I note that ISO C does not cast the return value of malloc() or realloc(). So should we cast the return value of malloc() or not? |
Generally speaking, I think it is widely accepted that casts damage type safety of a code base because they prevent tools from checking types ever again. (e.g., you could accidentally cast any type to a pointer type; not just from a pointer to void to some other pointer type.) A lot of coding standards ban redundant type casts for that reason, even before we get to their negative effects on the expressiveness of the language and the readability of code. I think that C++'s type system is fundamentally broken in this respect: void * has one purpose, polymorphism, and they broke it. MEM02-C proposes this pattern: widget *p; p = (widget *)malloc(sizeof(widget)); Which is only effective if someone checks by eye that the two instances of 'widget' in the same line match. Having to manually check for matches is never sign of robust code. Ideally, robust code should continue to work (or fail to compile) if either of the instances of 'widget' is changed. A more expressive and equally idiomatic alternative is this: widget *p; p = malloc(sizeof(*p)); This works even if p is a pointer to an array type. Admittedly, it still requires that the type of the operand to sizeof is correct with respect to the type of the lvalue to which the result of malloc is assigned. I have a lot more sympathy for the following suggested pattern, although I have never used it nor seen it used in any project: #define MALLOC(type) ((type *)malloc(sizeof(type))) However, I think adding such complexity could detract from the clarity of examples that are meant to demonstrate examples of UB (presumably as succinctly as possible). |
I don't have any strong opinion on which indentation style should be used except that it should be consistent (ideally both self-consistent and consistent with the ISO C standard). Since I have never seen 1TBS used in any project, it never occurred to me that it might be a deliberate choice. Had I suspected that, I never would have bothered changing it. I cannot see much consistency within the C standard, so I do not think that can be an authoritative guide on what to do. For example, in n3550 6.7.4.1 'General' and likewise through into 6.7.5 'Function specifiers' none of the example code snippets use 1TBS. Personally, I don't see much reason to depart from K&R's style but I don't feel strongly either way so I will reformat my commit according to your preferences. |
|
Chris: I agree that using sizeof() inside malloc() is often a better solution, but it is not perfect. See CERT guideline ARR01-C for an example: The type cast is useful if you are allocating a different type, as is shown in the first code code example, where p is a widget* but gets allocated as a gadget*. In the code example, this is bad...but if gadget was meant to be a "subclass" of widget, then it could be useful. This is often done in TCP code for example. There are two questions here: (1) should we revise or update MEM02-C, and (2) should we comply with MEM02-C in the examples document? They may very well have different answers. |
|
Chris: Which simply means that C23 is not consistent with brace styles (it has K&R and 1TBS code samples), and it not m uch of an authoritative source on brace styles. CERT C is more consistent :) |
|
Please see reworked and separated changes in #3 |
2 participants
Several commits:
Modified use of whitespace in examples where necessary to unify the style of the
document and match the style used in K&R's book. Braces are placed at the start
of a line when they enclose a function definition. Asterisks are placed such
that they accurately reflect the grammar of the language (in which * is part of
a declarator, not part of the declaration-specifiers).
Fixed errors and style of example 157: _file instead of *file, use of 0 as a
null pointer constant, use of undefined macro IOFBF and a redundant check for
buf == NULL. Substituted a read of the buffer for a write, because that seems
like a more straightforward example of using the contents of the array.
Fixed example 104. The example comprised function definitions rather than
declarations, and both had external linkage.
Removed redundant casts of the result of malloc and realloc.
Fixed another instance of misuse of _ in place of *.
Stop using 0 as a null pointer constant, for clarity.
Clarify the rules for reserved identifiers in example 105.