If you are targeting both C and C++, you do want to cast. If it's just C, the advice is not to cast because a) no point in repeating yourself, and b) it could mask an error if malloc was not declared.
a) is hardly an issue in a one-off macro definition, and b) is hardly an issue as you probably have #include <stdlib.h> just above the define. Besides, sane CFLAGS and any remotely modern compiler will protect you from b) anyway.
There is a minor benefit to casting in this case, as it will error out if I do:
int *x = new(x, 10); // should be new(int, 10)
because x is not a type.
However, I still think this is terrible advice because it turns something very standard that every beginner C programmer understands into something that requires familiarity with the codebase and incurs a constant cognitive load. It also makes for some very hard to debug errors if I use "new" as a variable name somewhere. Not to mention total incompatibility with C++ due to choice of name.
If you're #defining a macro, the cast is sensible because the macro did the allocation for that particular type => having the macro result cast protects against/warns when mistakenly assigning it to a pointer of different type.
The #define new has a , instead of multiplication. It will compile likely without warning, but cause buffer overflow.
says “use sprintf”, which you shouldn’t do because ... buffer overflow (it’s possible to avoid but you need to really know what you are doing and set a maximum length for any conversion, which article authors does not do). For the same use cases, Proper use of snprintf would avoid overflows.
There are several instances of bad syntax in the article. Easy to make mistakes if you don't write C daily. I assume they meant to use calloc() here instead of malloc().
I'm very much not a fan of indiscriminately typedef'ing structs. To me, a typedef describes a type as a "fundamental atom". Most structs are ... not that.
FWIW I also apply the above meaning for typedefs of integer types. It's an indicator for a "stronger" API boundary, i.e. you shouldn't mess with the contents.
Also this "Int" typedef in the article triggers a punch reflex for me. Would give it a hard NAK in any type of review. (Because it would lead to more WTFs in long-term maintenance of the code.)
sprintf(target, "%s%s", s1, s2);
Now, why would one not just use good old strcat? Well, there are three reasons:
- This is faster. Its about 20% faster than strcat,
since it accesses the string stored in target with a
SYSCALL instead of looping over it. I have checked
using GCC 9.3 on Manjaro with kernel 5.4 (Linux).
WTF? sprintf is not a syscall. Also, syscalls make things slower, not faster.
10 comments
[ 5.4 ms ] story [ 29.5 ms ] thread> #define new(dtype,length) (dtype *)malloc(sizeof(dtype),(length))
Kind of a minor point, but isn't the consensus among C programmers that you shouldn't cast the result of malloc?
The “don’t cast” consensus is about NULL remaining 0L and not of any pointer type (as was the case in some standard libraries in the early 90s)
Note also that the comma should be an asterisk.
a) is hardly an issue in a one-off macro definition, and b) is hardly an issue as you probably have #include <stdlib.h> just above the define. Besides, sane CFLAGS and any remotely modern compiler will protect you from b) anyway.
There is a minor benefit to casting in this case, as it will error out if I do:
because x is not a type.However, I still think this is terrible advice because it turns something very standard that every beginner C programmer understands into something that requires familiarity with the codebase and incurs a constant cognitive load. It also makes for some very hard to debug errors if I use "new" as a variable name somewhere. Not to mention total incompatibility with C++ due to choice of name.
says “use sprintf”, which you shouldn’t do because ... buffer overflow (it’s possible to avoid but you need to really know what you are doing and set a maximum length for any conversion, which article authors does not do). For the same use cases, Proper use of snprintf would avoid overflows.
Other advice seems ok to me.
FWIW I also apply the above meaning for typedefs of integer types. It's an indicator for a "stronger" API boundary, i.e. you shouldn't mess with the contents.
Also this "Int" typedef in the article triggers a punch reflex for me. Would give it a hard NAK in any type of review. (Because it would lead to more WTFs in long-term maintenance of the code.)
?!?