Verstable
Verstable copied to clipboard
Avoid discarding const.
Fixes #6.
Regarding
- table->metadata = (uint16_t *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( table ) );
+ table->metadata = (void *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( table ) );
and
- new_table.metadata = (uint16_t *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( &new_table ) );
+ new_table.metadata = (void *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( &new_table ) );
was Clang-Tidy complaining about this cast for some reason? The uint16_t * cast is necessary for C++ compatibility (as C++ disallows implicit conversion from void * to another pointer type).
And regarding
- static const uint16_t vt_empty_placeholder_metadatum = VT_EMPTY;
+ static uint16_t vt_empty_placeholder_metadatum = VT_EMPTY;
is this because Clang-Tidy complains about the (deliberate) casting-away of const in e.g. the _init function (table->metadata = (uint16_t *)&vt_empty_placeholder_metadatum;)? I'd rather leave vt_empty_placeholder_metadatum as const in order to make it very clear that the value of this "placeholder" is never changed. So maybe // NOLINT would be more appropriate here?
Regarding
- table->metadata = (uint16_t *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( table ) ); + table->metadata = (void *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( table ) );and
- new_table.metadata = (uint16_t *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( &new_table ) ); + new_table.metadata = (void *)( (unsigned char *)allocation + VT_CAT( NAME, _metadata_offset )( &new_table ) );was Clang-Tidy complaining about this cast for some reason? The
uint16_t *cast is necessary for C++ compatibility (as C++ disallows implicit conversion fromvoid *to another pointer type).
No, this was clang itself compiling it. I think clang17, or whatever is in Alpine Linux 3.20.0. It's necessary because char has an alignment of 1 and uint16_t an alignment of 2 so if it's off by one, it's a segfault on some platforms. On platforms which are sensitive to alignment it matters and clang warns you about it regardless of platform. The correct way (afaik) of telling a char pointer is correctly aligned to it's uint16_t pointer is to do an intermediate cast to void pointer. On C at least, you don't need an explicit cast of void * to any other pointer, hence (uint16_t *)(void *) is unnecessary. C++, I don't know as I don't use it.
And regarding
- static const uint16_t vt_empty_placeholder_metadatum = VT_EMPTY; + static uint16_t vt_empty_placeholder_metadatum = VT_EMPTY;is this because Clang-Tidy complains about the (deliberate) casting-away of
constin e.g. the_initfunction (table->metadata = (uint16_t *)&vt_empty_placeholder_metadatum;)? I'd rather leavevt_empty_placeholder_metadatumasconstin order to make it very clear that the value of this "placeholder" is never changed. So maybe// NOLINTwould be more appropriate here?
Actually it was gcc complaining about it. const is just a clue for correctness. You can unconst a pointer and write to it regardless. Dangerous if the pointer is a static assignment, but that is effectively what you are doing here because metadata where you are assigning to is changeable.
So both compiler errors. I'm on holiday for a week and don't have access to my sources right now, but from memory these are my CFLAGS.
CFLAGS+= -g -Wall -Wextra -Wundef
CFLAGS+= -Wmissing-prototypes -Wmissing-declarations
CFLAGS+= -Wmissing-format-attribute -Wnested-externs
CFLAGS+= -Wcast-align -Wcast-qual -Wpointer-arith
CFLAGS+= -Wreturn-type -Wswitch -Wshadow
CFLAGS+= -Wcast-qual -Wwrite-strings
CFLAGS+= -Wformat=2
CFLAGS+= -Wpointer-sign -Wmissing-noreturn
CFLAGS+= -Werror
I think I included -pedantic and -std=c23 as well, but can't be sure.
const is just a clue for correctness. You can unconst a pointer and write to it regardless. Dangerous if the pointer is a static assignment, but that is effectively what you are doing here because metadata where you are assigning to is changeable.
If the const variable is global, I believe compilers will typically store it in the program’s read-only “text” section. This is very desirable in this scenario, wherein the table’s metadata member could point to actual dynamically allocated metadata or this placeholder. It means that the program should crash if some bug causes a hash-table to attempt to write the the placeholder.
from memory these are my CFLAGS
Oh, I see. The issue here is the use of -Wcast-qual (in conjunction with -Werror). I've opened a new issue concerning support for optional diagnostics beyond -Wall -Wextra -Wpedantic (the diagnostics I already used when developing the library).