Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Remove useless casts to (void *)

  1. Remove useless casts to (void *)

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-11-20T13:33:51Z

    Hi hackers,

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

Meaning, the ones in the attached patch in:

- inval.c
- bump.c
- injection_point.c
- copyfromparse.c
- extension.c
- wparser.c

I did not find any reasons in the thread as to why those ones were not removed.

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

[1]: https://www.postgresql.org/message-id/flat/461ea37c-8b58-43b4-9736-52884e862820%40eisentraut.org
[2]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/remove_useless_casts_to_void_star.cocci

Regards,

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

  2. Re: Remove useless casts to (void *)

    Aleksander Alekseev <aleksander@tigerdata.com> — 2025-11-20T14:01:49Z

    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

  3. Re: Remove useless casts to (void *)

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-11-20T14:44:07Z

    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

  4. Re: Remove useless casts to (void *)

    Aleksander Alekseev <aleksander@tigerdata.com> — 2025-11-20T15:07:35Z

    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

  5. Re: Remove useless casts to (void *)

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-11-20T15:43:58Z

    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/

  6. Re: Remove useless casts to (void *)

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-11-20T17:13:15Z

    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], so spent some time to be able
to detect kind of automatically those useless casts.

[1]: https://www.postgresql.org/message-id/flat/461ea37c-8b58-43b4-9736-52884e862820@eisentraut.org

Regards,

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

  7. Re: Remove useless casts to (void *)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2025-11-20T18:16:19Z

    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

  8. Re: Remove useless casts to (void *)

    Peter Eisentraut <peter@eisentraut.org> — 2025-11-21T15:57:47Z

    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