Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <00d245b2-c0b5-9978-7f7b-9d5aa5df6137@loongson.cn>
Date: Fri, 17 Mar 2023 16:41:26 +0800
From: 王洪亮 <wanghongliang@...ngson.cn>
To: musl@...ts.openwall.com
Subject: Re: add loongarch64 port v6.


在 2023/2/17 下午3:06, 王洪亮 写道:
>
> 在 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

Hi, Rich

Do I need to do any other work with this patch v6?

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.