Re: Remove useless pointer advance in StatsShmemInit()
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-12-02T08:11:54Z
Lists: pgsql-hackers
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
Commits
-
Remove useless pointer update in StatsShmemInit()
- 9b7eb6f02e8d 19 (unreleased) landed