public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Thomas Munro <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Remove useless casts to (void *)
Date: Mon, 02 Dec 2024 20:36:13 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+hUKG+NFKnr=K4oybwDvT35dW=VAjAAfiuLxp+5JeZSOV3nBg@mail.gmail.com>
References: <[email protected]>
	<CA+hUKG+NFKnr=K4oybwDvT35dW=VAjAAfiuLxp+5JeZSOV3nBg@mail.gmail.com>

Thomas Munro <[email protected]> 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

view thread (6+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Remove useless casts to (void *)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox