[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Merging scratch/no-purespace to remove unexec and purespace
From: |
Stefan Kangas |
Subject: |
Re: Merging scratch/no-purespace to remove unexec and purespace |
Date: |
Thu, 19 Dec 2024 12:55:01 -0500 |
Pip Cet <pipcet@protonmail.com> writes:
> "Stefan Kangas" <stefankangas@gmail.com> writes:
>
>> Pip Cet <pipcet@protonmail.com> writes:
>>
>>> What I think we should do doesn't really matter, but it seems quite
>>> obvious to me that we should make the code on the master branch
>>> perform all three checks on all relocations, as the code on
>>> no-purespace does.
>>
>> Maybe. But won't we get those checks with no additional effort once we
>> merge no-purespace,
>
> Yes, we will. (And the forbidden symbol; even if the forbidden symbol
> doesn't cause trouble, which I think it will, it's simply very poor
> programming practice to do things that way, particularly since the
> crash may happen a long time after the compilation. But, again, what I
> think obviously doesn't matter here. I'll just remember that
> --enable-checking causes false positive crashes and shouldn't be used).
I don't think the existence of one symbol that will crash Emacs in some
situations means that --enable-checking should be completely avoided.
It's still quite useful, and we're fine as long as we avoid using that
one symbol, right?
OTOH and IMHO, it would be preferable if that symbol could not crash
Emacs. Can we come up with a good way to fix that, while preserving the
check that Andrea wants to keep?
> That's a problem, because if we run into problems there, we'll have no
> way of knowing whether the problem was present on the pre-merge master
> (where we didn't check) or not. But, again, as it's specific to
> --enable-checking, we can simply stop using that.
>
>> and if so, can't it wait until then?
>
> Of course, but changing two things at a time makes debugging harder.
> (And IIUC, we won't rename the symbol on master until we merge, so
> that's three changes which can cause trouble with the nativecomp code,
> all introduced at the same time).
I still don't think I understand your argument here, sorry.
The scratch/no-purespace branch contains several different changes, all
of which have had to pass through review, testing and verification.
Why is it particularly important to "backport" this change to master, in
advance of the merge, but not any of the other changes on that branch?
What am I missing?
- Re: Merging scratch/no-purespace to remove unexec and purespace, (continued)
- Re: Merging scratch/no-purespace to remove unexec and purespace, Pip Cet, 2024/12/18
- Re: Merging scratch/no-purespace to remove unexec and purespace, Andrea Corallo, 2024/12/18
- Re: Merging scratch/no-purespace to remove unexec and purespace, Pip Cet, 2024/12/19
- Re: Merging scratch/no-purespace to remove unexec and purespace, Andrea Corallo, 2024/12/19
- Re: Merging scratch/no-purespace to remove unexec and purespace, Stefan Kangas, 2024/12/19
- Re: Merging scratch/no-purespace to remove unexec and purespace, Pip Cet, 2024/12/19
- Re: Merging scratch/no-purespace to remove unexec and purespace,
Stefan Kangas <=
- Re: Merging scratch/no-purespace to remove unexec and purespace, Pip Cet, 2024/12/19
- Re: Merging scratch/no-purespace to remove unexec and purespace, Stefan Monnier, 2024/12/20
- Re: Merging scratch/no-purespace to remove unexec and purespace, Pip Cet, 2024/12/20
- Re: Merging scratch/no-purespace to remove unexec and purespace, Gerd Möllmann, 2024/12/20
- Re: Merging scratch/no-purespace to remove unexec and purespace, Andrea Corallo, 2024/12/20
- Re: Merging scratch/no-purespace to remove unexec and purespace, Pip Cet, 2024/12/21
- Re: Merging scratch/no-purespace to remove unexec and purespace, Gerd Möllmann, 2024/12/21
- Re: Merging scratch/no-purespace to remove unexec and purespace, Andrea Corallo, 2024/12/21
- Re: Merging scratch/no-purespace to remove unexec and purespace, Pip Cet, 2024/12/21
- Re: Merging scratch/no-purespace to remove unexec and purespace, Gerd Möllmann, 2024/12/20