linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Daniel Latypov <dlatypov@google.com>
Cc: "David Gow" <davidgow@google.com>,
	"Justin Stitt" <justinstitt@google.com>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"Guenter Roeck" <linux@roeck-us.net>,
	"Rae Moar" <rmoar@google.com>,
	"Matthew Auld" <matthew.auld@intel.com>,
	"Arunpravin Paneer Selvam" <arunpravin.paneerselvam@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Maíra Canal" <mcanal@igalia.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Matthew Brost" <matthew.brost@intel.com>,
	"Willem de Bruijn" <willemb@google.com>,
	"Florian Westphal" <fw@strlen.de>,
	"Cassio Neri" <cassio.neri@gmail.com>,
	"Javier Martinez Canillas" <javierm@redhat.com>,
	"Arthur Grillo" <arthur.grillo@usp.br>,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"David Airlie" <airlied@gmail.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	intel-xe@lists.freedesktop.org, linux-rtc@vger.kernel.org,
	linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
	linux-hardening@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg
Date: Thu, 22 Feb 2024 09:56:40 -0800	[thread overview]
Message-ID: <CAHk-=wiODww51Kz-TTWn0ka5T8oMtt0AfbO9t0U3iJqfLZO+8w@mail.gmail.com> (raw)
In-Reply-To: <CAGS_qxoW0v0eM646zLu=SWL1O5UUp5k08SZsQO51gCDx_LnhcQ@mail.gmail.com>

On Thu, 22 Feb 2024 at 09:36, Daniel Latypov <dlatypov@google.com> wrote:
>
> Copying the line for context, it's about `p-r` where
>   p = memchr_inv(&r[1], 0, sizeof(r) - sizeof(r[0]));
> `p-r` should never be negative unless something has gone horribly
> horribly wrong.

Sure it would - if 'p' is NULL.

Of course, then a negative value wouldn't be helpful either, and in
this case that's what the EXPECT_PTR_EQ checking is testing in the
first place, so it's a non-issue.

IOW, in practice clearly the sign should simply not matter here.

I do think that the default case for pointer differences should be
that they are signed, because they *can* be.

Just because of that "default case", unless there's some actual reason
to use '%tu', I think '%td' should be seen as the normal case to use.

That said, just as a quick aside: be careful with pointer differences
in the kernel.

For this particular case, when we're talking about just 'char *', it's
not a big deal, but we've had code where people didn't think about
what it means to do a pointer difference in C, and how it can be often
unnecessarily expensive due to the implied "divide by the size of the
pointed object".

Sometimes it's actually worth writing the code in ways that avoids
pointer differences entirely (which might involve passing around
indexes instead of pointers).

                 Linus

  reply	other threads:[~2024-02-22 17:57 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-21  9:27 [PATCH 0/9] kunit: Fix printf format specifier issues in KUnit assertions David Gow
2024-02-21  9:27 ` [PATCH 1/9] kunit: test: Log the correct filter string in executor_test David Gow
2024-02-21 13:25   ` Guenter Roeck
2024-02-21 20:04   ` Justin Stitt
2024-02-21 20:29   ` Daniel Latypov
2024-02-22 20:58   ` Rae Moar
2024-02-21  9:27 ` [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg David Gow
2024-02-21 13:25   ` Guenter Roeck
2024-02-21 20:10   ` Justin Stitt
2024-02-22  6:22     ` David Gow
2024-02-22 17:36       ` Daniel Latypov
2024-02-22 17:56         ` Linus Torvalds [this message]
2024-02-21  9:27 ` [PATCH 3/9] lib: memcpy_kunit: " David Gow
2024-02-21 13:25   ` Guenter Roeck
2024-02-21 21:05   ` Justin Stitt
2024-02-21  9:27 ` [PATCH 4/9] time: test: Fix incorrect format specifier David Gow
2024-02-21 13:25   ` Guenter Roeck
2024-02-21 21:06   ` Justin Stitt
2024-02-21  9:27 ` [PATCH 5/9] rtc: test: Fix invalid " David Gow
2024-02-21 13:26   ` Guenter Roeck
2024-02-21 21:06   ` Justin Stitt
2024-02-27 20:32   ` Alexandre Belloni
2024-02-27 21:23     ` Shuah Khan
2024-02-27 22:48       ` Alexandre Belloni
2024-02-21  9:27 ` [PATCH 6/9] net: test: Fix printf format specifier in skb_segment kunit test David Gow
2024-02-21 13:26   ` Guenter Roeck
2024-02-21 21:26   ` Justin Stitt
2024-02-21  9:27 ` [PATCH 7/9] drm: tests: Fix invalid printf format specifiers in KUnit tests David Gow
2024-02-21 10:21   ` Matthew Auld
2024-02-21 10:45   ` Christian König
2024-02-21 13:26   ` Guenter Roeck
2024-02-21 21:29   ` Justin Stitt
2024-02-27 23:24     ` Shuah Khan
2024-02-21  9:27 ` [PATCH 8/9] drm/xe/tests: Fix printf format specifiers in xe_migrate test David Gow
2024-02-21 13:26   ` Guenter Roeck
2024-02-22  5:05   ` Lucas De Marchi
2024-02-22  5:59     ` Linus Torvalds
2024-02-22  9:52   ` Thomas Hellström
2024-02-21  9:27 ` [PATCH 9/9] kunit: Annotate _MSG assertion variants with gnu printf specifiers David Gow
2024-02-21 13:26   ` Guenter Roeck
2024-02-21 20:02   ` Justin Stitt
2024-02-22 14:23 ` [PATCH 0/9] kunit: Fix printf format specifier issues in KUnit assertions Shuah Khan
2024-02-27 23:32 ` Shuah Khan

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='CAHk-=wiODww51Kz-TTWn0ka5T8oMtt0AfbO9t0U3iJqfLZO+8w@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=airlied@gmail.com \
    --cc=arthur.grillo@usp.br \
    --cc=arunpravin.paneerselvam@amd.com \
    --cc=brendan.higgins@linux.dev \
    --cc=cassio.neri@gmail.com \
    --cc=christian.koenig@amd.com \
    --cc=davem@davemloft.net \
    --cc=davidgow@google.com \
    --cc=dlatypov@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fw@strlen.de \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=javierm@redhat.com \
    --cc=justinstitt@google.com \
    --cc=keescook@chromium.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=matthew.auld@intel.com \
    --cc=matthew.brost@intel.com \
    --cc=mcanal@igalia.com \
    --cc=mripard@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rmoar@google.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=sboyd@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=willemb@google.com \
    /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;
as well as URLs for NNTP newsgroup(s).