[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5d5caaea-4031-44a7-932e-2cf3f6fa4d6e@nfschina.com>
Date: Mon, 17 Mar 2025 09:33:42 +0800
From: Su Hui <suhui@...china.com>
To: Elizabeth Figura <zfigura@...eweavers.com>,
Dan Carpenter <dan.carpenter@...aro.org>
Cc: shuah@...nel.org, wine-devel@...ehq.org, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 1/4] selftests: ntsync: fix the wrong condition in
wake_all
On 2025/3/16 04:29, Elizabeth Figura wrote:
> On Saturday, 15 March 2025 04:39:46 CDT Dan Carpenter wrote:
>> On Fri, Mar 14, 2025 at 05:13:50PM -0500, Elizabeth Figura wrote:
>>> On Friday, 14 March 2025 05:14:30 CDT Su Hui wrote:
>>>> On 2025/3/14 17:21, Dan Carpenter wrote:
>>>>> On Fri, Mar 14, 2025 at 03:14:51PM +0800, Su Hui wrote:
>>>>>> When 'manual=false' and 'signaled=true', then expected value when using
>>>>>> NTSYNC_IOC_CREATE_EVENT should be greater than zero. Fix this typo error.
>>>>>>
>>>>>> Signed-off-by: Su Hui<suhui@...china.com>
>>>>>> ---
>>>>>> tools/testing/selftests/drivers/ntsync/ntsync.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
>>>>>> index 3aad311574c4..bfb6fad653d0 100644
>>>>>> --- a/tools/testing/selftests/drivers/ntsync/ntsync.c
>>>>>> +++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
>>>>>> @@ -968,7 +968,7 @@ TEST(wake_all)
>>>>>> auto_event_args.manual = false;
>>>>>> auto_event_args.signaled = true;
>>>>>> objs[3] = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &auto_event_args);
>>>>>> - EXPECT_EQ(0, objs[3]);
>>>>>> + EXPECT_LE(0, objs[3]);
>>>>> It's kind of weird how these macros put the constant on the left.
>>>>> It returns an "fd" on success. So this look reasonable. It probably
>>>>> won't return the zero fd so we could probably check EXPECT_LT()?
>>>> Agreed, there are about 29 items that can be changed to EXPECT_LT().
>>>> I can send a v2 patchset with this change if there is no more other
>>>> suggestions.
>>> I personally think it looks wrong to use EXPECT_LT(), but I'll certainly
>>> defer to a higher maintainer on this point.
>> I'm not sure I understand what you are saying. Are you saying that we
>> should allow zero as an expected file descriptor here? I don't have
>> strong feelings about that either way.
> Yes, my apologies for the ambiguous wording. That is, EXPECT_LE looks more correct to me than EXPECT_LT per se.
Got it, I think there is no need for v2 patch that using EXPECT_LT().
Thanks for your feedback.
Su Hui
Powered by blists - more mailing lists