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.
