Re: Remove useless casts to (void *)

Thomas Munro <thomas.munro@gmail.com> writes:
> 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.

I think getting rid of Pointer altogether would cause a lot of code
churn for not that much benefit; most places are (I think) just using
it as a name for a generic pointer.  I could support a patch to
define it as "void *" not "char *", if there aren't too many places
that would have to be modified.

> DatumGetPointer() should arguably return void *
> too, to force users to provide a type if they're going to dereference
> or perform arithmetic.

Well, it returns Pointer, which is what it should return.

> 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?

The same goes for a number of other historical macros, such as
OidIsValid.  I think there was possibly once an idea that super-duper
debug builds could apply stricter tests in these macros.  Nothing's
ever been done in that line, but that doesn't make it an awful idea.
I don't particularly object to code that just checks "if (x)", but
I wouldn't be in favor of removing these macros, if only because the
sheer size of the patch would make it a back-patching land mine.

> 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.

Probably.  Again, it'd be interesting to try changing it and see how
invasive the patch winds up being.

> (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 :-))

Yeah.  I think that "ptr = palloc(n * sizeof(char))" is good practice
as documentation, even when we all know "sizeof(char)" is 1.

			regards, tom lane