logoalt Hacker News

unwindlast Tuesday at 12:42 PM1 replyview on HN

Quite interesting, and felt fairly "modern" (which for C programming advice sometimes only means it's post-2000 or so). A few comments:

----

This:

    struct Vec3* v = malloc(sizeof(struct Vec3));
is better written as:

    struct Vec3 * const v = malloc(sizeof *v);
The `const` is perhaps over-doing it, but it makes it clear that "for the rest of this scope, the value of this pointer won't change" which I think is good for readability. The main point is "locking" the size to the size of the type being pointed at, rather than "freely" using `sizeof` the type name. If the type name later changes, or `Vec4` is added and code is copy-pasted, this lessens the risk of allocating the wrong amount and is less complicated.

----

This is maybe language-lawyering, but you can't write a function named `strclone()` unless you are a C standard library implementor. All functions whose names begin with "str" followed by a lower-case letter are reserved [1].

----

This `for` loop header (from the "Use utf8 strings" section:

    for (size_t i = 0; *str != 0; ++len)
is just atrocious. If you're not going to use `i`, you don't need a `for` loop to introduce it. Either delete (`for(; ...` is valid) or use a `while` instead.

----

In the "Zero Your Structs" section, it sounds as if the author recommends setting the bits of structures to all zero in order to make sure any pointer members are `NULL`. This is dangerous, since C does not guarantee that `NULL` is equivalent to all-bits-zero. I'm sure it's moot on modern platforms where implementations have chosen to represent `NULL` as all-bits-zero, but that should at least be made clear.

[1]: https://www.gnu.org/software/libc/manual/html_node/Reserved-...


Replies

jandreselast Tuesday at 5:18 PM

    This:

    struct Vec3* v = malloc(sizeof(struct Vec3));

    is better written as:

    struct Vec3 * const v = malloc(sizeof *v);
I don't love this. Other people are going to think you're only allocating a pointer. It's potentially confusing.
show 2 replies