[PATCH] kcsan: Use min() to fix Coccinelle warning

[Thorsten Blum]

Fixes the following Coccinelle/coccicheck warning reported by
minmax.cocci:

WARNING opportunity for min()

Use size_t instead of int for the result of min().

Signed-off-by: Thorsten Blum <thorst...@toblux.com>
---
kernel/kcsan/debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
index 1d1d1b0e4248..11b891fe6f7a 100644
--- a/kernel/kcsan/debugfs.c
+++ b/kernel/kcsan/debugfs.c
@@ -225,7 +225,7 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
{
char kbuf[KSYM_NAME_LEN];
char *arg;
- int read_len = count < (sizeof(kbuf) - 1) ? count : (sizeof(kbuf) - 1);
+ size_t read_len = min(count, (sizeof(kbuf) - 1));

if (copy_from_user(kbuf, buf, read_len))
return -EFAULT;
--
2.45.2

[Marco Elver]

On Mon, 24 Jun 2024 at 00:08, Thorsten Blum <thorst...@toblux.com> wrote:
>
> Fixes the following Coccinelle/coccicheck warning reported by
> minmax.cocci:
>
> WARNING opportunity for min()
>
> Use size_t instead of int for the result of min().
>
> Signed-off-by: Thorsten Blum <thorst...@toblux.com>

Reviewed-by: Marco Elver <el...@google.com>

Thanks for polishing (but see below). Please compile-test with
CONFIG_KCSAN=y if you haven't.

> ---
> kernel/kcsan/debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
> index 1d1d1b0e4248..11b891fe6f7a 100644
> --- a/kernel/kcsan/debugfs.c
> +++ b/kernel/kcsan/debugfs.c
> @@ -225,7 +225,7 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
> {
> char kbuf[KSYM_NAME_LEN];
> char *arg;
> - int read_len = count < (sizeof(kbuf) - 1) ? count : (sizeof(kbuf) - 1);
> + size_t read_len = min(count, (sizeof(kbuf) - 1));

While we're here polishing things this could be:

const size_t read_len = min(count, sizeof(kbuf) - 1);

( +const, remove redundant () )

[Thorsten Blum]

On 24. Jun 2024, at 00:02, Marco Elver <el...@google.com> wrote:
> On Mon, 24 Jun 2024 at 00:08, Thorsten Blum <thorst...@toblux.com> wrote:
>>
>> Fixes the following Coccinelle/coccicheck warning reported by
>> minmax.cocci:
>>
>> WARNING opportunity for min()
>>
>> Use size_t instead of int for the result of min().
>>
>> Signed-off-by: Thorsten Blum <thorst...@toblux.com>
>
> Reviewed-by: Marco Elver <el...@google.com>
>
> Thanks for polishing (but see below). Please compile-test with
> CONFIG_KCSAN=y if you haven't.

Yes, I compile-tested it with CONFIG_KCSAN=y, but forgot to mention it.

> While we're here polishing things this could be:
>
> const size_t read_len = min(count, sizeof(kbuf) - 1);
>
> ( +const, remove redundant () )

Should I submit a v2 or are you adding this already?

Thanks,
Thorsten

[Marco Elver]

Sending a v2 is cleaner, and also Cc Paul E. McKenney
<pau...@kernel.org>, because the KCSAN patches go through the -rcu
kernel tree.

Thanks,
-- Marco

[Thorsten Blum]

Fixes the following Coccinelle/coccicheck warning reported by
minmax.cocci:

WARNING opportunity for min()

Use const size_t instead of int for the result of min().

Compile-tested with CONFIG_KCSAN=y.

Reviewed-by: Marco Elver <el...@google.com>

Signed-off-by: Thorsten Blum <thorst...@toblux.com>
---

Changes in v2:
- Add const and remove redundant parentheses as suggested by Marco Elver

---
kernel/kcsan/debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c

index 1d1d1b0e4248..53b21ae30e00 100644

--- a/kernel/kcsan/debugfs.c
+++ b/kernel/kcsan/debugfs.c
@@ -225,7 +225,7 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
{
char kbuf[KSYM_NAME_LEN];
char *arg;
- int read_len = count < (sizeof(kbuf) - 1) ? count : (sizeof(kbuf) - 1);

+ const size_t read_len = min(count, sizeof(kbuf) - 1);

[David Laight]

From: Marco Elver
> Sent: 24 June 2024 08:03

> >
> > Fixes the following Coccinelle/coccicheck warning reported by
> > minmax.cocci:
> >
> > WARNING opportunity for min()
> >
> > Use size_t instead of int for the result of min().
> >
> > Signed-off-by: Thorsten Blum <thorst...@toblux.com>
>
> Reviewed-by: Marco Elver <el...@google.com>
>
> Thanks for polishing (but see below). Please compile-test with
> CONFIG_KCSAN=y if you haven't.
>
> > ---
> > kernel/kcsan/debugfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
> > index 1d1d1b0e4248..11b891fe6f7a 100644
> > --- a/kernel/kcsan/debugfs.c
> > +++ b/kernel/kcsan/debugfs.c
> > @@ -225,7 +225,7 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
> > {
> > char kbuf[KSYM_NAME_LEN];
> > char *arg;
> > - int read_len = count < (sizeof(kbuf) - 1) ? count : (sizeof(kbuf) - 1);
> > + size_t read_len = min(count, (sizeof(kbuf) - 1));
>
> While we're here polishing things this could be:
>
> const size_t read_len = min(count, sizeof(kbuf) - 1);
>
> ( +const, remove redundant () )

Pretty much no one makes variables 'const', it mostly just makes the code harder to read.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[Marco Elver]

This is very much subjective. In my subjective opinion, it makes the
code easier to understand and it'll be harder to introduce accidental
mistakes. For trivial cases like this it really doesn't matter though.

[Marco Elver]

On Sun, 30 Jun 2024 at 22:03, Thorsten Blum <thorst...@toblux.com> wrote:
>
> Fixes the following Coccinelle/coccicheck warning reported by
> minmax.cocci:
>
> WARNING opportunity for min()
>
> Use size_t instead of int for the result of min().
>

> Compile-tested with CONFIG_KCSAN=y.
>
> Reviewed-by: Marco Elver <el...@google.com>
> Signed-off-by: Thorsten Blum <thorst...@toblux.com>
> ---
> Changes in v2:
> - Add const and remove redundant parentheses as suggested by Marco Elver

> - Link to v1: https://lore.kernel.org/linux-kernel/20240623220606.134...@toblux.com/
>
> Changes in v3:
> - Remove const again after feedback from David Laight

I think I was clear that the removal of const was not needed in this
case, and my preference was to keep const.

While general and _constructive_ comments are helpful and appreciated,
this level of nit-picking and bikeshedding about 'const' is a complete
and utter waste of time. I'm sorry, but I'm rather allergic to this
level of time-wasting.

As KCSAN maintainer, I'm just going to say I prefer v2.

> - Link to v2: https://lore.kernel.org/linux-kernel/20240624175727.880...@toblux.com/

[+Cc Paul]

Paul, if possible kindly pick v2 of this patch into the KCSAN tree:
https://lore.kernel.org/linux-kernel/20240624175727.880...@toblux.com/

Many thanks,
-- Marco

[Paul E. McKenney]

I have queued v2 of this patch, which is as shown below. Please let me
know if anything needs adjustment. If things go well, this should make
the upcoming merge window.

Thanx, Paul

------------------------------------------------------------------------

commit 613b072fe9b3aa11410937498c98b7ac6d7c9d5a
Author: Thorsten Blum <thorst...@toblux.com>
Date: Mon Jun 24 19:57:28 2024 +0200

kcsan: Use min() to fix Coccinelle warning

Fixes the following Coccinelle/coccicheck warning reported by
minmax.cocci:

WARNING opportunity for min()

Use const size_t instead of int for the result of min().

Compile-tested with CONFIG_KCSAN=y.

Reviewed-by: Marco Elver <el...@google.com>
Signed-off-by: Thorsten Blum <thorst...@toblux.com>

Signed-off-by: Paul E. McKenney <pau...@kernel.org>

diff --git a/kernel/kcsan/debugfs.c b/kernel/kcsan/debugfs.c
index 1d1d1b0e4248..53b21ae30e00 100644

--- a/kernel/kcsan/debugfs.c
+++ b/kernel/kcsan/debugfs.c
@@ -225,7 +225,7 @@ debugfs_write(struct file *file, const char __user *buf, size_t count, loff_t *o
{
char kbuf[KSYM_NAME_LEN];
char *arg;
- int read_len = count < (sizeof(kbuf) - 1) ? count : (sizeof(kbuf) - 1);