Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 16 Feb 2023 18:13:37 -0500
From: Rich Felker <dalias@...c.org>
To: 王洪亮 <wanghongliang@...ngson.cn>
Cc: musl@...ts.openwall.com
Subject: Re: add loongarch64 port v6.

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

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.