Re: Remove useless pointer advance in StatsShmemInit()

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Remove useless pointer advance in StatsShmemInit()
Date: 2025-12-02 08:11:54
Message-ID: aS6fSjWon-BaOf8v@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2025-12-02 08:21:34 Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp
Previous Message Antonin Houska 2025-12-02 08:07:54 Re: Index functions and INDEX_CREATE_SKIP_BUILD