Re: Remove useless pointer advance in StatsShmemInit()

Hi,

On 2025-12-03 08:14:41 +0000, Bertrand Drouvot wrote:
> On Tue, Dec 02, 2025 at 03:00:39PM -0500, Andres Freund wrote:
> > On 2025-12-02 07:40:44 +0000, Bertrand Drouvot wrote:
> > > From 2fefb69f1462ce1057bb5c3d07ed70c769ec961a Mon Sep 17 00:00:00 2001
> > > From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
> > > Date: Sat, 22 Nov 2025 14:47:25 +0000
> > > Subject: [PATCH v1] Remove useless pointer updates
> > >
> > > Same idea as in commit 9b7eb6f02e8. Those pointers are updated but are not used
> > > after the updates, so let's remove the useless updates or document why we want
> > > to keep them.
> >
> > What's the point of this? Compilers are perfectly capable of removing a
> > trailing store if the updated value isn't ever used afterwards.
>
> yeah, but my motivation isn't execution efficiency but rather code clarity and
> maintenance burden.
>
> I'm proposing to "remove the useless updates or document why we want
> to keep them". If the consensus is to keep them all, that's fine, but I think
> we should at least add a comment. That would avoid:
>
> - people spending time trying to understand why these updates exist, only to
> eventually realize it's dead code, and potentially sending unnecessary cleanup
> patches.
>
> - code (including the dead code) being copy/pasted into new patches where the
> increment might actually cause bugs or confusion. FWIW, that's exactly how I
> discovered the one in 9b7eb6f02e8 (during a patch review where this code was
> copy/pasted).
>
> So, what about documenting them all?

I'm -0.1 on that. I think it's just as likely that those code comments will
not be moved when another chunk of memory is needed and cause confusion that
way. Sorry, but to me this is just a case of restating obvious code in an
obvious mechanical comment.

Greetings,

Andres Freund