Re: Remove useless casts to (void *)

On Tue, Oct 29, 2024 at 9:06 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> 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.

Some other places that sprang to my eye recently that reminded me of
K&R C and the ongoing transition to the standard (/me ducks):

Why do we bother with a "Pointer" type? The idiomatic way to do
byte-oriented pointer arithmetic is to cast to char *, or uint8_t*
etc, which doesn't require the reader to go and figure out what stride
that non-standard type name implies. Clearly it has a history linked
to the introduction of void *, and it's described as 'holding the
address of any memory resident type' and is filed under the 'standard
system types' section, but C89 has void * for objects of type unknown
to the compiler, and if you want to do arithmetic over those objects
you have to be more explicit about their size, which requires writing
a suitably-alarming-to-the-reader cast to a pointer to a real type.
Also, the explanation about "true ANSI compilers" means nothing to the
modern reader who wasn't hacking C 30+ years ago.

Maybe it has to do with matching up to DatumGetPointer().
DatumGetPointer() isn't defined to return it though, it returns char
*, callers usually/always cast to a real time, and I don't see Pointer
being used near it. DatumGetPointer() should arguably return void *
too, to force users to provide a type if they're going to dereference
or perform arithmetic.

What problem does PointerIsValid(x) solve, when you could literally
just write x or if you insist x != NULL in most contexts and it would
be 100% idiomatic C, and shorter? Why introduce extra terminological
load with "validity"? C programmers had better know all about NULL.

Why do all the XLogRegister*() calls have to cast their argument to
char *? Seems like a textbook use for void * argument, not requiring
a cast. If the XLog* interfaces want to do byte-oriented arithmetic
internally, they should do the cast internally, instead of having an
argument that requires all callers to cast their arguments.

(While grepping for casts to char *, I had a mistake in my regex and
finished up seeing how many places in our code check sizeof(char),
which is funny because sizeof is defined as telling you how many chars
it takes to hold a type/value; perhaps it has documentation value :-))