Remove useless casts to (void *)

Hi Bertrand,

The attached also remove casts that have been added since 7f798aca1d5, the ones
in pg_publication.c, lock.c and tuplesortvariants.c.

The patch has been generated with the help of the .cocci script [2] (though I
manually reviewed and removed some matches that, I think, were not appropriate).

I didn't review the entire patch but one change caught my attention:

```
-               databuf = (void *) ((char *) databuf + avail);
+               databuf = (char *) databuf + avail;
```

Here `databuf` has a type (void*). Although the code is correct, it
replaces an explicit cast (which I read "yes, we know what we are
doing") with an implicit one. Personally I don't think this is a good
change.

These were just my two cents. All in all I'm neither for nor against the patch.

--
Best regards,
Aleksander Alekseev

Hi Aleksander,

On Thu, Nov 20, 2025 at 05:01:49PM +0300, Aleksander Alekseev wrote:

Hi Bertrand,

The attached also remove casts that have been added since 7f798aca1d5, the ones
in pg_publication.c, lock.c and tuplesortvariants.c.

The patch has been generated with the help of the .cocci script [2] (though I
manually reviewed and removed some matches that, I think, were not appropriate).

I didn't review the entire patch but one change caught my attention:

Thanks for looking at it!

```
-               databuf = (void *) ((char *) databuf + avail);
+               databuf = (char *) databuf + avail;
```

Here `databuf` has a type (void*). Although the code is correct, it
replaces an explicit cast (which I read "yes, we know what we are
doing") with an implicit one.

Yes, that's what it is doing and so was 7f798aca1d5.

For example in 7f798aca1d5, you can also see things like:

@@ -858,7 +858,7 @@ setup_firstcall(FuncCallContext *funcctx, HStore *hs,
st = (HStore *) palloc(VARSIZE(hs));
memcpy(st, hs, VARSIZE(hs));

-       funcctx->user_fctx = (void *) st;
+       funcctx->user_fctx = st;

Where funcctx->user_fctx is of type (void *) and st is of type (HStore *).

Personally I don't think this is a good change.

Only this one (because maybe databuf is used twice) or the whole idea of 7f798aca1d5
and this patch?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Hi,

Personally I don't think this is a good change.

Only this one (because maybe databuf is used twice) or the whole idea of 7f798aca1d5
and this patch?

The whole idea to be honest. This being said, this is just my personal
opinion as of today. The majority of the community may disagree and/or
have good arguments to do this I'm not aware of or failed to
understand and/or just have other sense of beauty. And that's OK. IMO
what actually is important is for the code to be consistent. That's as
I understand what your patch is trying to accomplish.

Let's see what other people think.

--
Best regards,
Aleksander Alekseev

On 2025-Nov-20, Aleksander Alekseev wrote:

IMO what actually is important is for the code to be consistent.

IMO what is most important is that the code is correct. Second most
important is that the code is performant. The consistency is perhaps a
third priority, if that -- there may be other objectives that are also
more important than consistency.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

Hi,

On Thu, Nov 20, 2025 at 04:43:58PM +0100, �lvaro Herrera wrote:

On 2025-Nov-20, Aleksander Alekseev wrote:

IMO what actually is important is for the code to be consistent.

IMO what is most important is that the code is correct. Second most
important is that the code is performant. The consistency is perhaps a
third priority, if that -- there may be other objectives that are also
more important than consistency.

Yeah, I was convinced by the discussion in [1]/messages/by-id/461ea37c-8b58-43b4-9736-52884e862820@eisentraut.org, so spent some time to be able
to detect kind of automatically those useless casts.

[1]: /messages/by-id/461ea37c-8b58-43b4-9736-52884e862820@eisentraut.org

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Thu, Nov 20, 2025 at 6:02 AM Aleksander Alekseev
<aleksander@tigerdata.com> wrote:

Here `databuf` has a type (void*). Although the code is correct, it
replaces an explicit cast (which I read "yes, we know what we are
doing") with an implicit one.

"Yes, we know what we are doing" is the argument against doing it,
though. There's no upside to telling the compiler that in this case:
if it's correct, the compiler would have done it for you anyway, and
if it's buggy, now the compiler has been told to stay silent.

So +1 on removing unneeded (void *) casts in general, for the sake of
establishing consensus, though I haven't looked at this particular
patch in detail.

--Jacob

On 20.11.25 14:33, Bertrand Drouvot wrote:

I was working on something similar to [1] (which led to 7f798aca1d5) and I think
that 7f798aca1d5 missed removing some useless casts to (void *).

committed, thanks