uthash.h: fix compiler warning -Wcast-qual

[obgm]

The macro HASH_ADD_KEYPTR_BYHASHVALUE casts keyptr to (char ) hence violating const keys. As the key is used read-only, the UT_hash_handle structure can be changed to const void key, allwowing the cast to (const void *) instead.

This fixes compiler warnings emitted when -Wcast-qual is specified.

[obgm]

To reproduce:

#include "uthash.h"

int main(void) {
  struct obj  {
    UT_hash_handle hh;
    const int field;
  } add, *hashtable = NULL;

  HASH_ADD(hh, hashtable, field, sizeof(add.field), &add);
}

$ cc -Wcast-qual -c tst.c 
In file included from tst.c:1:
tst.c: In function ‘main’:
uthash.h:411:19: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
  411 |   (add)->hh.key = (char*) (keyptr);                                              \
      |                   ^
uthash.h:432:3: note: in expansion of macro ‘HASH_ADD_KEYPTR_BYHASHVALUE’
  432 |   HASH_ADD_KEYPTR_BYHASHVALUE(hh, head, keyptr, keylen_in, _ha_hashv, add);      \
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
uthash.h:439:3: note: in expansion of macro ‘HASH_ADD_KEYPTR’
  439 |   HASH_ADD_KEYPTR(hh, head, &((add)->fieldname), keylen_in, add)
      |   ^~~~~~~~~~~~~~~
tst.c:9:3: note: in expansion of macro ‘HASH_ADD’
    9 |   HASH_ADD(hh, hashtable, field, sizeof(add.field), &add);
      |   ^~~~~~~~

[Quuxplusone]

Seems like a good idea. However, the diffs in this PR aren't sufficient. Run cd tests ; make EXTRA_CFLAGS=-Wcast-qual to see what fails. For example:

test6.c:99:9: warning: passing 'const void *' to parameter of type 'void *' discards qualifiers
      [-Wincompatible-pointer-types-discards-qualifiers]
        HASH_FIND_INT(users,&i,tmp);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~

../src/utarray.h:235:25: warning: cast from 'const void *' to 'char **' drops const qualifier [-Wcast-qual]
  char **_src = (char**)src, **_dst = (char**)dst;
                        ^

It would also be good to know which compiler(s) support the -Wcast-qual option, and whether it's enabled by -Wall (it's not), by -Wextra (?), and/or by -pedantic.

[obgm]

I have pushed a set of commits to address warnings from the unit tests. If you like, I can squash them, of course.

[obgm]

Apparently, neither -Wall, nor -Wpedantic, nor -Wextra enable this warning (-Wcast-qual). However, -Wextra causes more warnings for the unit tests (due to unused parameters for the main function and signedness).

[obgm]

A quick survey using the Godbolt compiler explorer yields the following result.
tldr; most recent compilers seem to accept this option except for Microsoft's native compiler.

Compiler	Version	-Wcast-qual Support	Remark
gcc	4.7+	✔
gcc	4.6.4	❌
gcc	4.5	❌
clang	5+	✔
clang	4	❌
djgpp	4.9.4+	✔
ellcc	2017-07-16	✔
ellcc	0.1.34	❌
icc	13.0.1	✔	versions below not tested
AT	12.0	✔	Powerpc, other versions not tested
msvc	v19.21	❌
zapcc	190308	❌

[Quuxplusone]

Merged 9673de849ca526f29bed7a93731367d048af25aa a7f6b2d1769a32a0bf7123cae4c55a13e50b6d0e bdddc2468d2bf6cf1833ecc7bae3a1f6f2aba689; please rebase.

I'm still pondering the remaining change in uthash.h — I mean it seems harmless, but I'm giving it some time in case I come up with some reason not to do it. If this PR gathers no further comments by May 30, ping me and I'll merge it. :)

[obgm]

Rebased.

[Quuxplusone]

Hi, I just went to look at merging this, and found that master still has some -Wcast-qual diagnostics in the tests even after these two remaining commits.

cc  -I../src -g -Wall -Wcast-qual -pedantic -DHASH_BLOOM=16 -DHASH_DEBUG -DNO_DECLTYPE  -o test87 test87.c
test87.c:22:5: warning: cast from 'const hstruct_t *' to 'char *' drops const qualifier [-Wcast-qual]
    HASH_ITER(hh, hTable, search, tmp) {
    ^
../src/uthash.h:1070:31: note: expanded from macro 'HASH_ITER'
for(((el)=(head)), ((*(char**)(&(tmp)))=(char*)((head!=NULL)?(head)->hh.next:NULL)); \
                              ^
test87.c:22:5: warning: cast from 'const hstruct_t *' to 'char *' drops const qualifier [-Wcast-qual]
../src/uthash.h:1071:42: note: expanded from macro 'HASH_ITER'
  (el) != NULL; ((el)=(tmp)), ((*(char**)(&(tmp)))=(char*)((tmp!=NULL)?(tmp)->hh.next:NULL)))
                                         ^

Adding const in the two obvious places causes a different warning:

cc  -I../src -g -Wall -Wcast-qual -pedantic -DHASH_BLOOM=16 -DHASH_DEBUG -DNO_DECLTYPE  -o test15 test15.c
test15.c:35:5: warning: cast from 'struct my_struct **' to 'const char **' must have all intermediate pointers const qualified to be safe
      [-Wcast-qual]
    HASH_ITER(hh, users, s, tmp) {
    ^

test46 and test52 are just dumb on purpose:

c++ -x c++  -I../src -g -Wall -Wcast-qual -pedantic  -o test52 test52.c
test52.c:34:17: warning: cast from 'const char *' to 'char *' drops const qualifier [-Wcast-qual]
    ic.s=(char*)"hello";
                ^

Do you happen to know if there's any accepted way to suppress -Wcast-qual in this case? or should we spend the lines-of-code to rewrite this in some way that doesn't need to cast away const?

[obgm]

The intermediate const cast is a tough one. I am not sure how to fix these properly.

Regarding test46 and test52, it would be easy to fix these by adding const to the declaration and removing the cast for the assigment.

A way to drop the compiler flag for specific tests would be to include the following (however, I am not sure if and how this needs to be done for other compilers such as the Windows C compiler):

#if defined(__clang__) || defined(__GNUC__)
#pragma GCC diagnostic ignored "-Wcast-qual"
#endif

[Quuxplusone]

A way to drop the compiler flag for specific tests would be to include the following

Agreed, I'd prefer to stay away from #pragma GCC diagnostic.

[Quuxplusone]

Merged ba2fbfdcd8dda139357c8b3ea4fdd13ea599f6b6 (which I tweaked a bit as long as we're modifying those lines) and 0831d9a08730f4625f90e790f31089e34b83f9ae.