Re: Remove useless casting to the same type

Hi,

On Fri, Nov 28, 2025 at 09:11:16AM +0100, Peter Eisentraut wrote:
> On 25.11.25 06:46, Bertrand Drouvot wrote:
> > > > @@ -389,7 +389,7 @@ hash_xlog_split_allocate_page(XLogReaderState *record)
> > > >
> > > > /* extract low and high masks. */
> > > > memcpy(&lowmask, data, sizeof(uint32));
> > > > - highmask = (uint32 *) ((char *) data + sizeof(uint32));
> > > > + highmask = (uint32 *) (data + sizeof(uint32));
> > > I wonder about these, too. I like knowing what the code does without
> > > having to check the type of `data`. But then later on we do a `data +=
> > > sizeof(uint32) * 2`, so you have to check the type anyway, so... I
> > > don't know.
> > I think that even with the cast in place, it's good to check the type of data.
> > Not for the line that follows (i.e: "data += sizeof(uint32) * 2") but to check
> > that the cast makes sense and does not hide "wrong" pointer manipulation.
> >
> > So I think that with or without the cast one would need to check. But that feels
> > more natural to check when there is no cast (as we don't assume that someone
> > said "I know what I'm doing"). So I'm in favor of removing the cast, thoughts?
>
> I think this whole thing could be simplified by overlaying a uint32 over
> "data" and just accessing the array fields normally. See attached patch.

Indeed, that's a nice simplification.

- data += sizeof(uint32) * 2;

Is it safe? I mean could XLH_SPLIT_META_UPDATE_MASKS and XLH_SPLIT_META_UPDATE_SPLITPOINT
be set simultaneously?

Regards,

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