Re: scratch/igc abefefd4f88: Add a staticpro in search.c (bug#75459)

Re: scratch/igc abefefd4f88: Add a staticpro in search.c (bug#75459)

[Stefan Kangas]

Gerd Moellmann <gerd.moellmann@gmail.com> writes:

> branch: scratch/igc
> commit abefefd4f887c8b282da4a848367ab89fe17ce41
> Author: Gerd Möllmann <gerd@gnu.org>
> Commit: Gerd Möllmann <gerd@gnu.org>
>
>     Add a staticpro in search.c (bug#75459)
>
>     * src/search.c (syms_of_search): Staticpro a Lisp_Object member in
>     searchbufs for HAVE_MPS.
> ---
>  src/search.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/search.c b/src/search.c
> index 36a932b015b..98d11131e31 100644
> --- a/src/search.c
> +++ b/src/search.c
> @@ -3446,6 +3446,11 @@ syms_of_search (void)
>        staticpro (&searchbufs[i].regexp);
>        staticpro (&searchbufs[i].f_whitespace_regexp);
>        staticpro (&searchbufs[i].syntax_table);
> +#ifdef HAVE_MPS
> +      /* Strictly speaking, this is also necessary for the old GC,
> +	 unless one would prove it isn't.  Don't rock the boat.  */
> +      staticpro (&searchbufs[i].buf.translate);
> +#endif

Is there any harm from doing this for the old GC also?

Re: scratch/igc abefefd4f88: Add a staticpro in search.c (bug#75459)

[Gerd Möllmann]

Stefan Kangas <stefankangas@gmail.com> writes:

> Gerd Moellmann <gerd.moellmann@gmail.com> writes:
>
>> branch: scratch/igc
>> commit abefefd4f887c8b282da4a848367ab89fe17ce41
>> Author: Gerd Möllmann <gerd@gnu.org>
>> Commit: Gerd Möllmann <gerd@gnu.org>
>>
>>     Add a staticpro in search.c (bug#75459)
>>
>>     * src/search.c (syms_of_search): Staticpro a Lisp_Object member in
>>     searchbufs for HAVE_MPS.
>> ---
>>  src/search.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/src/search.c b/src/search.c
>> index 36a932b015b..98d11131e31 100644
>> --- a/src/search.c
>> +++ b/src/search.c
>> @@ -3446,6 +3446,11 @@ syms_of_search (void)
>>        staticpro (&searchbufs[i].regexp);
>>        staticpro (&searchbufs[i].f_whitespace_regexp);
>>        staticpro (&searchbufs[i].syntax_table);
>> +#ifdef HAVE_MPS
>> +      /* Strictly speaking, this is also necessary for the old GC,
>> +	 unless one would prove it isn't.  Don't rock the boat.  */
>> +      staticpro (&searchbufs[i].buf.translate);
>> +#endif
>
> Is there any harm from doing this for the old GC also?

No :-)

Re: scratch/igc abefefd4f88: Add a staticpro in search.c (bug#75459)

[Stefan Kangas]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>>> diff --git a/src/search.c b/src/search.c
>>> index 36a932b015b..98d11131e31 100644
>>> --- a/src/search.c
>>> +++ b/src/search.c
>>> @@ -3446,6 +3446,11 @@ syms_of_search (void)
>>>        staticpro (&searchbufs[i].regexp);
>>>        staticpro (&searchbufs[i].f_whitespace_regexp);
>>>        staticpro (&searchbufs[i].syntax_table);
>>> +#ifdef HAVE_MPS
>>> +      /* Strictly speaking, this is also necessary for the old GC,
>>> +	 unless one would prove it isn't.  Don't rock the boat.  */
>>> +      staticpro (&searchbufs[i].buf.translate);
>>> +#endif
>>
>> Is there any harm from doing this for the old GC also?
>
> No :-)

Thanks, that is my understanding also, so I made this unconditional.
It seems both simpler and more correct.

Maybe we should cherry-pick it to master even, but I'll leave
that to someone else.

Re: scratch/igc abefefd4f88: Add a staticpro in search.c (bug#75459)

[Gerd Möllmann]

Stefan Kangas <stefankangas@gmail.com> writes:

> Thanks, that is my understanding also, so I made this unconditional.
> It seems both simpler and more correct.

👍

Re: scratch/igc abefefd4f88: Add a staticpro in search.c (bug#75459)

[Pip Cet via Emacs development discussions.]

"Stefan Kangas" <stefankangas@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Stefan Kangas <stefankangas@gmail.com> writes:
>>
>>>> diff --git a/src/search.c b/src/search.c
>>>> index 36a932b015b..98d11131e31 100644
>>>> --- a/src/search.c
>>>> +++ b/src/search.c
>>>> @@ -3446,6 +3446,11 @@ syms_of_search (void)
>>>>        staticpro (&searchbufs[i].regexp);
>>>>        staticpro (&searchbufs[i].f_whitespace_regexp);
>>>>        staticpro (&searchbufs[i].syntax_table);
>>>> +#ifdef HAVE_MPS
>>>> +      /* Strictly speaking, this is also necessary for the old GC,
>>>> +	 unless one would prove it isn't.  Don't rock the boat.  */
>>>> +      staticpro (&searchbufs[i].buf.translate);
>>>> +#endif
>>>
>>> Is there any harm from doing this for the old GC also?
>>
>> No :-)
>
> Thanks, that is my understanding also, so I made this unconditional.

Thanks!

> It seems both simpler and more correct.

I think it's a jaywalking bug: it may be safe to do that once in a
while, but that doesn't make it legal.

This specific instance seems safe to me, but I spent too much time
trying to establish that it was, and I still might have missed
something.  And all that effort only (semi-)established that it's safe
right now, but that might change for entirely different reasons.

> Maybe we should cherry-pick it to master even, but I'll leave
> that to someone else.

Any objections?

Pip