Re: Remove useless pointer advance in StatsShmemInit() - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Remove useless pointer advance in StatsShmemInit()
Date
Msg-id aS6fSjWon-BaOf8v@paquier.xyz
Whole thread Raw
In response to Re: Remove useless pointer advance in StatsShmemInit()  (Bertrand Drouvot <bertranddrouvot.pg@gmail.com>)
List 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

Attachment

pgsql-hackers by date:

Previous
From: Antonin Houska
Date:
Subject: Re: Index functions and INDEX_CREATE_SKIP_BUILD
Next
From: Amit Langote
Date:
Subject: Re: Refactoring: Use soft error reporting for *_opt_overflow functions of date/timestamp