Some simple tricks in C/C++ | Hacker News Reader

Some simple tricks in C/C++

able.bio

12 pointsnew_here10 comments

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.

?!?

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.

Other advice seems ok to me.

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().

> Slap this on the top of your code:

> #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?

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.

On the contrary. you must cast it or get a warning in modern compilers (and an error if you compile it in C++ mode).

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.

It's fine in C and you won't get a warning (just try int *p = malloc(sizeof(int)) with gcc -Wall).

I really doubt any compilers will warn on void* casts in pure C. Usually the warning is if you do:

   int *x;
   char *y = x;

or similar. But if x was "void*", it really should work on all C compilers.

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.

Reply on news.ycombinator.com