Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 17 Feb 2023 15:06:35 +0800
From: 王洪亮 <wanghongliang@...ngson.cn>
To: musl@...ts.openwall.com
Subject: Re: add loongarch64 port v6.


在 2023/2/17 上午7:13, Rich Felker 写道:
> On Mon, Jan 30, 2023 at 09:27:21AM +0800, 王洪亮 wrote:
>> 在 2023/1/30 上午1:04, Rich Felker 写道:
>>> On Sun, Jan 29, 2023 at 03:52:18AM -0500, Ariadne Conill wrote:
>>>> Hi,
>>>>
>>>> On 2023-01-28 20:15, 王洪亮 wrote:
>>>>> Hi,
>>>>>
>>>>> Is there anything else that needs to be modified in patch v6?
>>>> It is likely that you will get a better review if you split up
>>>> the changes into logical ones and submit them to the list as
>>>> a group of patches.
>>> It's been a while since I looked in detail, but I don't think that's
>>> necessary here. It should just be a matter of ensuring that all
>>> previously requested changes were made, and that the ABI is
>>> official/stable on kernel and compiler sides. I've been away overseas
>>> (on vacation) for the past month and this kind of review is outside
>>> the scope of what I've been checking in on while away, but I will be
>>> back in roughly a week.
>>>
>>> Rich
>> Yes,this patch is modified according to the previous suggestions,
>> and the ABI is consistent with the kernel and compiler sides.
>>
>> I'm looking forward to your review and reply in a week.thanks.
> One thing that's come up since previous review is that we had things
> wrong around the kernel sigaction ABI on a number of archs. From the
> way you defined SA_RESTORER as 0, it looks like loongarch64 is
> intended not to have a restorer member in the kernel sigaction
> structure. Can you confirm? I think this means the ksigaction you're
> using in the musl port right now is wrong and mismatched with the
> kernel. If my understanding is right, once my patches for fixing the
> other archs are pushed, just removing the #define SA_RESTORER 0 line
> will make this correct.
>
> As long as the kernel has officially decided on adopting __NR_clone,
> it's fine (and preferable) to stick with using it for __clone.
>
> I still see a few places with whitespace issues here and there but I
> don't want to waste your time with them; I can clean them up in the
> diff before applying it.
>
> I also spotted some minor namespace details in bits/signal.h. I don't
> think this needs to block merge. I can prepare/propose a patch on top
> of the one adding the arch.
>
> So, really I think the only thing I need right now is to know whether
> my understanding of the SA_RESTORER situation is correct.
>
> Rich
Yes, your understanding is correct. I have confirmed that there is no 
SA_RESTORER
define in loongarch64, so no sa_restorer member in kernel sigaction.

Hongliang Wang

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.