On Tue, Dec 02, 2025 at 07:40:44AM +0000, Bertrand Drouvot wrote:
> The reason is that, while they are currently useless, they would need to be
> added back if we add more branches/cases. So I preferred to stay on the safe side
> of thing.
@@ -75,7 +75,6 @@ heap_xlog_prune_freeze(XLogReaderState *record)
/* memcpy() because snapshot_conflict_horizon is stored unaligned */
memcpy(&snapshot_conflict_horizon, maindataptr, sizeof(TransactionId));
- maindataptr += sizeof(TransactionId);
I'd rather leave this one untouched. maindataptr is initialized at
the beginning of the routine.
left_hikey = (IndexTuple) datapos;
left_hikeysz = MAXALIGN(IndexTupleSize(left_hikey));
- datapos += left_hikeysz;
datalen -= left_hikeysz;
This one looks intentional to me, in line with datalen.
@@ -1960,7 +1960,6 @@ DecodeXLogRecord(XLogReaderState *state,
out = (char *) MAXALIGN(out);
decoded->main_data = out;
memcpy(decoded->main_data, ptr, decoded->main_data_len);
- ptr += decoded->main_data_len;
out += decoded->main_data_len;
This one is definitely intentional, and I've looked at this code a lot.
/* account for alignment */
ring_mem_remain -= ring_mem_next - shmem;
- shmem += ring_mem_next - shmem;
-
- shmem += ring_mem_remain;
I'd leave these ones as well, except if its author argues for changing
it. It is documentation.
The one in gin_desc() is pointless, indeed. This looks like a remnant
of 5dc851afde8d to me.
#if defined(WAIT_USE_EPOLL)
set->epoll_ret_events = (struct epoll_event *) data;
- data += MAXALIGN(sizeof(struct epoll_event) * nevents);
#elif defined(WAIT_USE_KQUEUE)
set->kqueue_ret_events = (struct kevent *) data;
- data += MAXALIGN(sizeof(struct kevent) * nevents);
#elif defined(WAIT_USE_POLL)
set->pollfds = (struct pollfd *) data;
- data += MAXALIGN(sizeof(struct pollfd) * nevents);
#elif defined(WAIT_USE_WIN32)
set->handles = (HANDLE) data;
- data += MAXALIGN(sizeof(HANDLE) * nevents);
#endif
There is an argument about block reordering on this one, where we'd
still want data to be incremented to a maxaligned area if we read more
data past the number of events.
Not sure about the ones in ReorderBufferSerializeChange(). There's an
effort done so as the computations could still matter if the code is
reordered or refactored for a reason or another.
--
Michael