From: Willy Tarreau <w@1wt.eu>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] selftests/nolibc: add 7 tests for memcmp()
Date: Thu, 27 Oct 2022 19:20:41 +0200 [thread overview]
Message-ID: <20221027172041.GB30081@1wt.eu> (raw)
In-Reply-To: <0b8feeb2-6ec6-d2af-8aa7-0bf34e7ab4b2@rasmusvillemoes.dk>
Hi Rasmus,
On Thu, Oct 27, 2022 at 11:09:55AM +0200, Rasmus Villemoes wrote:
> >> While you're at it, may I suggest also adding a few test cases where the
> >> buffers differ by 128, e.g. 0x0 v 0x80 and 0x40 v 0xc0.
> >
> > I initially thought about it but changed my mind for +/- 0xc0 that
> > covered the same cases in my opinion. Do you have a particular error
> > case in mind that would be caught by this one that the other one does
> > not catch ?
>
> Not really, but in a sense the opposite: for the +/- 0xc0 case, both
> ways of comparison will give the wrong sign because -192 becomes +64 and
> vice versa. For +/- 0x80, one way of doing the comparison will
> "accidentally" produce the right answer, and I thought that might also
> be a little interesting.
OK, initially I thought you were trying to make the comparison return a
match when there is none. I now see better what you mean there.
> > I'm fine for proposing a respin of the patch to improve
> > it if it brings some value,
>
> It's your call, you can respin, do an incremental patch, or just ignore
> me :)
I would like to propose you something. Till now I'm the only one having
added tests to this file, and I'm still lacking feedback on the usability.
I would very much appreciate it if you could try to add this test case
yourself on top of existing ones (those present in Paul's rcu/next branch
here:
https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/ ).
Then your criticism of what you would find unclear, unconvenient, poorly
thought, unintuitive etc, and of course suggestions, would be welcome.
That doesn't mean I'd have a quick solution of course but the more eyes
there at the early stages, the better so that it becomes friendly to use
for other contributors. If you don't want to, that's no big deal, but if
you do I'll really appreciate it.
Thank you,
Willy
next prev parent reply other threads:[~2022-10-27 17:20 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-21 6:03 [PATCH] selftests/nolibc: add 7 tests for memcmp() Willy Tarreau
2022-10-21 15:56 ` Paul E. McKenney
2022-10-21 17:01 ` Willy Tarreau
2022-10-21 17:07 ` Paul E. McKenney
2022-10-21 17:20 ` Willy Tarreau
2022-10-21 18:00 ` Paul E. McKenney
2022-10-22 11:22 ` Willy Tarreau
2022-10-24 15:53 ` Paul E. McKenney
2022-10-26 5:39 ` Willy Tarreau
2022-10-26 9:08 ` Rasmus Villemoes
2022-10-26 19:52 ` Willy Tarreau
2022-10-27 9:09 ` Rasmus Villemoes
2022-10-27 17:20 ` Willy Tarreau [this message]
2022-10-26 13:57 ` Paul E. McKenney
2022-10-26 14:17 ` Willy Tarreau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221027172041.GB30081@1wt.eu \
--to=w@1wt.eu \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=paulmck@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox