From: David Gow <davidgow@google.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Shuah Khan <shuah@kernel.org>,
Brendan Higgins <brendanhiggins@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Kees Cook <keescook@chromium.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>,
kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH linux-kselftest/test v4] lib/list-test: add a test for the 'list' doubly linked list
Date: Tue, 22 Oct 2019 15:13:28 -0700 [thread overview]
Message-ID: <CABVgOSkegmhmeRa=7Qcx3MnX88wLy9qZx97CMhk4NvWb-pgpYQ@mail.gmail.com> (raw)
In-Reply-To: <20191019082731.GM21344@kadam>
On Sat, Oct 19, 2019 at 1:27 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Fri, Oct 18, 2019 at 02:55:49PM -0700, David Gow wrote:
> > + list4 = kzalloc(sizeof(*list4), GFP_KERNEL);
> > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, list4);
>
> Why not just use GFP_KERNEL | GFP_NOFAIL and remove the check?
I've sent a new version of the patch out (v5) which uses __GFP_NOFAIL instead.
The idea had been to exercise KUnit's assertion functionality, in the
hope that it'd allow the test to fail (but potentially allow other
tests to still run) in the case of allocation failure. Given that
we're only allocating enough to store ~4 pointers in total, though,
that's probably of little use.
> kzalloc() can't return error pointers. If this were an IS_ERR_OR_NULL()
> check then it would generate a static checker warning, but static
> checkers don't know about KUNIT_ASSERT_NOT_ERR_OR_NULL() yet so you're
> safe.
Alas, KUnit doesn't have a KUNIT_ASSERT_NOT_NULL() macro, and I'd
assumed it was not dangerous (even if not ideal) to check for error
pointers, even if kzalloc() can't return them.
Perhaps it'd make sense to add a convenient way of checking the
NULL-ness of pointers to KUnit (it's possible with the
KUNIT_ASSERT_PTR_EQ(), but requires a bit of casting to make the type
checker happy) in the future. Once KUnit is properly upstream, it may
be worth teaching the static analysis tools about these functions to
avoid having warnings in these sorts of tests.
For now, though, (and for this test in particular), I agree with the
suggestion of just using __GFP_NOFAIL.
Thanks a lot for the comments,
-- David
next prev parent reply other threads:[~2019-10-22 22:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-18 21:55 David Gow
2019-10-19 8:27 ` Dan Carpenter
2019-10-22 22:13 ` David Gow [this message]
2019-10-23 21:25 ` Brendan Higgins
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='CABVgOSkegmhmeRa=7Qcx3MnX88wLy9qZx97CMhk4NvWb-pgpYQ@mail.gmail.com' \
--to=davidgow@google.com \
--cc=akpm@linux-foundation.org \
--cc=brendanhiggins@google.com \
--cc=dan.carpenter@oracle.com \
--cc=keescook@chromium.org \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=shuah@kernel.org \
--subject='Re: [PATCH linux-kselftest/test v4] lib/list-test: add a test for the '\''list'\'' doubly linked list' \
/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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).