From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Bremner <david@tethera.net>, Dick Marinus <dick@mrns.nl>,
 notmuch@notmuchmail.org
Subject: Re: [PATCH 1/2] CLI/reply: use static const for reply_to_map
In-Reply-To: <875x3cwzwi.fsf@tethera.net>
References: <20260617192542.3516741-1-dick@mrns.nl>
 <20260617192542.3516741-2-dick@mrns.nl> <y12ijeo.9lm4qp-too@iki.fi>
 <875x3cwzwi.fsf@tethera.net>
User-Agent: Notmuch/0.40+24~gc45207d (https://notmuchmail.org) Emacs/27.1
Date: Sun, 21 Jun 2026 23:26:40 +0300
Message-ID: <y12fl1e.i3l0ur-too@iki.fi>
MIME-Version: 1.0
Message-ID-Hash: 3LV5SQPABHASQ6LQXWCYDPFD5H5AOBNU
Precedence: list
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

On Sun, Jun 21 2026, David Bremner wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> On Wed, Jun 17 2026, Dick Marinus wrote:
>>
>>> It's recommended to store these functions in a const struct, for some
>>> reason I even got a segmentation fault without this.
>>
>> The change looks sane (I also checked all references to reply_to_map in
>> code). With this the array does not need to create on-the-fly in that
>> function but can (even) be created to read-only location at compile-time
>> (barring binding to dynamic libs when program starts but still 
>> mpu-protected...)
>>
>> But about the commit message. Who recommends this thing? you? god?
>> deepseek? someone(tm)? Looks somewhat vague statement to me.
>>
>> Then the sigsegv - I don't see why the code before that change would
>> sigsegv. Without more details so that it could be reproduced such
>> a statement should not be mentioned in commit message.
>
> I think this is probably the bug discussed in
>
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125252
>   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1135526
>   https://nmbug.notmuchmail.org/nmweb/show/87pl3e2nvl.fsf%40tethera.net
>
> (more information in the first link).
>
> I did not see anywhere in the bug log (but maybe I missed it) that the
> code was actually wrong. If we do decide to make the change to work
> around a gcc bug, we should be clear about that.

Thanks for the links, by going through those I could see that is exactly
the bug there. I also could reproduce it with gcc 16.1, -std=gnu23 and -O2
(and then w/o that -gnu23 as it is now default -- with -gnu11 no crash).

IMO the commit message could first tell that it makes the structure 
compile-time constant and often stored in read-only location and then
mention it happens to work around bug in gcc 15.2 and 16.1, just for
historical reference (and since gcc bug was found out trying to run
notmuch test suite with notmuch compiled with gcc 15.2)

(at least gcc 15.4 and 16.2 will have this fixed)

Tomi
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org