From: Nathan Chancellor <natechancellor@gmail.com>
To: Aleksa Sarai <cyphar@cyphar.com>
Cc: Christian Brauner <christian.brauner@ubuntu.com>,
Kees Cook <keescook@chromium.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] lib: test_user_copy: style cleanup
Date: Sat, 5 Oct 2019 22:01:37 -0700 [thread overview]
Message-ID: <20191006050137.GA1789703@archlinux-threadripper> (raw)
In-Reply-To: <20191005233028.18566-1-cyphar@cyphar.com>
On Sun, Oct 06, 2019 at 10:30:28AM +1100, Aleksa Sarai wrote:
> While writing the tests for copy_struct_from_user(), I used a construct
> that Linus doesn't appear to be too fond of:
>
> On 2019-10-04, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > Hmm. That code is ugly, both before and after the fix.
> >
> > This just doesn't make sense for so many reasons:
> >
> > if ((ret |= test(umem_src == NULL, "kmalloc failed")))
> >
> > where the insanity comes from
> >
> > - why "|=" when you know that "ret" was zero before (and it had to
> > be, for the test to make sense)
> >
> > - why do this as a single line anyway?
> >
> > - don't do the stupid "double parenthesis" to hide a warning. Make it
> > use an actual comparison if you add a layer of parentheses.
>
> So instead, use a bog-standard check that isn't nearly as ugly.
>
> Fixes: 341115822f88 ("usercopy: Add parentheses around assignment in test_copy_struct_from_user")
> Fixes: f5a1a536fa14 ("lib: introduce copy_struct_from_user() helper")
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
I assume the comment diff is a line length/alignment thing? The commit
message does not mention it.
Regardless, thank you for providing the fix that I should have.
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
next prev parent reply other threads:[~2019-10-06 5:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-05 23:30 [PATCH] lib: test_user_copy: style cleanup Aleksa Sarai
2019-10-06 5:01 ` Nathan Chancellor [this message]
2019-10-06 8:41 ` Christian Brauner
2019-10-06 10:47 ` Aleksa Sarai
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=20191006050137.GA1789703@archlinux-threadripper \
--to=natechancellor@gmail.com \
--cc=christian.brauner@ubuntu.com \
--cc=cyphar@cyphar.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.