3 ms·

I would agree on this for some cases (and I suspect more so for != NULL than != 0), but memcmp() != 0 isn't one of them. There are multiple reasons why, some mo

I would agree on this for some cases (and I suspect more so for != NULL than != 0), but memcmp() != 0 isn't one of them. There are multiple reasons why, some more generic than others, but two of them are very specific to memcmp(): 1. There are similar functions (like std::lexicographical_compare() in C++) returning bool instead an int. 2. It's not crazy to read memcmp() as comparing for equality rather than ordering. i.e. I think when you see 'if (memcmp(a, b, n))', it's quite natural for one's brain to read it as "if a and b are equal..." For someone who uses memcmp() on a daily basis, you probably don't encounter these misconceptions. But as someone who writes C++ and merely deals with C once in a while in the process, both of these possibilities have tripped me up multiple times when seeing if (memcmp(...)). Writing != 0 has always cleared that confusion for me, since it immediately signals the result (a) isn't meant to be read as a Boolean, and (b) might be negative, thus putting my brain on alert.

Stratoscope 4y ago
When I used to write a lot of C code, my solution for this was to avoid calling memcmp() directly at all when I only cared about a yes/no on the match. Instead I used a wrapper I called memeq(): int memeq( const void* lhs, const void* rhs, size_t count ) { return memcmp( lhs, rhs, count ) == 0; } One could have an analogous memneq() with != 0, but I preferred to just use the ! operator on the memeq() result. I would also use a typedef for the struct, so you don't have to keep writing "struct" everywhere: typedef struct { int x, y, z; } tmp; With those two tricks and more generous whitespace for clarity, the erroneous memcmp() call becomes: if( ! memeq( m_result_original, m_result_my_version, sizeof(tmp) ) ) You're a lot less likely to get that wrong. That said, this code, even with my updates, was wrong all along. Using memcmp() on two structs, even with a wrapper, is a bad idea. What if there is padding?! This particular struct will not have padding, but in the more general case you shouldn't count on that. What you really need to do is have a function that explicitly compares the x, y, and z fields of these two structs. Then you won't have to worry about uninitialized padding, you will never get the length wrong, and everything will always just work.
- jcelerier 4y ago
  Reading this makes me appreciate how nice it is in c++ to just be able to do operator==() = default; to let the compiler generate the appropriate comparator automatically

Stratoscope 4y ago

When I used to write a lot of C code, my solution for this was to avoid calling memcmp() directly at all when I only cared about a yes/no on the match. Instead I used a wrapper I called memeq(): int memeq( const void* lhs, const void* rhs, size_t count ) { return memcmp( lhs, rhs, count ) == 0; } One could have an analogous memneq() with != 0, but I preferred to just use the ! operator on the memeq() result. I would also use a typedef for the struct, so you don't have to keep writing "struct" everywhere: typedef struct { int x, y, z; } tmp; With those two tricks and more generous whitespace for clarity, the erroneous memcmp() call becomes: if( ! memeq( m_result_original, m_result_my_version, sizeof(tmp) ) ) You're a lot less likely to get that wrong. That said, this code, even with my updates, was wrong all along. Using memcmp() on two structs, even with a wrapper, is a bad idea. What if there is padding?! This particular struct will not have padding, but in the more general case you shouldn't count on that. What you really need to do is have a function that explicitly compares the x, y, and z fields of these two structs. Then you won't have to worry about uninitialized padding, you will never get the length wrong, and everything will always just work.

jcelerier 4y ago
Reading this makes me appreciate how nice it is in c++ to just be able to do operator==() = default; to let the compiler generate the appropriate comparator automatically