linux-kernel - Re: [PATCH 1/4] selftests: ntsync: fix the wrong condition in wake

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1961744.6tgchFWduM@camazotz>
Date: Sat, 15 Mar 2025 15:29:43 -0500
From: Elizabeth Figura <zfigura@...eweavers.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Su Hui <suhui@...china.com>, 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 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.

> Putting variables on the right, Yoda speak is.  Unnatural is.

Yes, I certainly agree with this. I wrote it this way in the first place because I was following some other example, I forget which.

> I did a git grep and the KUNIT_EXPECT_LT() just calls the parameters
> left and right instead of "expected" and "seen".  Expected is wrong
> for LT because we expect it to be != to the expected value.  It's
> the opposite.  We're expecting the unexpected!  It would be better
> to just call them left and right.
> 
> regards,
> dan carpenter
>