On 29.10.24 15:20, Tom Lane wrote:
Peter Eisentraut <pe...@eisentraut.org> writes:
There are a bunch of (void *) casts in the code that don't make sense to
me. I think some of these were once necessary because char * was used
in place of void * for some function arguments. And some of these were
probably just copied around without further thought. I went through and
cleaned up most of these. I didn't find any redeeming value in these.
They are just liable to hide actual problems such as incompatible types.
But maybe there are other opinions.
I don't recall details, but I'm fairly sure some of these prevented
compiler warnings on some (old?) compilers. Hard to be sure if said
compilers are all gone.
Looking at the sheer size of the patch, I'm kind of -0.1, just
because I'm afraid it's going to create back-patching gotchas.
I don't really find that it's improving readability, though
clearly that's a matter of opinion.
I did a bit of archeological research on these. None of these casts
were ever necessary, and in many cases even the original patch that
introduced an API used the coding style inconsistently. So I'm very
confident that there are no significant backward compatibility or
backpatching gotchas here.
I'm more concerned that many of these just keep getting copied around
indiscriminately, and this is liable to hide actual type mismatches or
silently discard qualifiers. So I'm arguing in favor of a more
restrictive style in this matter.
