All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: kernel test robot <lkp@intel.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>,
	kbuild-all@lists.01.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: mm/sparse.c:145:2: warning: int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn]
Date: Mon, 12 Jul 2021 20:41:45 +0100	[thread overview]
Message-ID: <YOya+aBZFFmC476e@casper.infradead.org> (raw)
In-Reply-To: <202107130348.6LsVT9Nc-lkp@intel.com>

On Tue, Jul 13, 2021 at 03:19:06AM +0800, kernel test robot wrote:
> cppcheck warnings: (new ones prefixed by >>)
> >> mm/sparse.c:145:2: warning: int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn]
>     return (nid << SECTION_NID_SHIFT);

Hrm.

include/linux/mmzone.h:#define SECTION_NID_SHIFT                3

I'm going to suggest that we don't allow for 2^29 node IDs in a single
system, so this doesn't actually represent a bug that needs to be fixed.
On the other hand, this type of bug continually bites us, and it would
be good to warn about this kind of thing.  So in the spirit of silencing
the warning by doing the promotion that C should have specified in
the first place ...

--- 8< ---

[PATCH] Avoid a warning in sparse memory support

cppcheck warns that we're possibly losing information by shifting an int.
It's a false positive, because we don't allow for a NUMA node ID that
large, but if we ever change SECTION_NID_SHIFT, it could become a problem,
and in any case this is usually a legitimate warning.  Fix it by adding
the necessary cast, which makes the compiler generate the right code.

diff --git a/mm/sparse.c b/mm/sparse.c
index 6326cdf36c4f..f17bd4f7caaa 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -143,7 +143,7 @@ unsigned long __section_nr(struct mem_section *ms)
  */
 static inline unsigned long sparse_encode_early_nid(int nid)
 {
-	return (nid << SECTION_NID_SHIFT);
+	return ((unsigned long)nid << SECTION_NID_SHIFT);
 }
 
 static inline int sparse_early_nid(struct mem_section *section)

WARNING: multiple messages have this Message-ID (diff)
From: Matthew Wilcox <willy@infradead.org>
To: kbuild-all@lists.01.org
Subject: Re: mm/sparse.c:145:2: warning: int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn]
Date: Mon, 12 Jul 2021 20:41:45 +0100	[thread overview]
Message-ID: <YOya+aBZFFmC476e@casper.infradead.org> (raw)
In-Reply-To: <202107130348.6LsVT9Nc-lkp@intel.com>

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

On Tue, Jul 13, 2021 at 03:19:06AM +0800, kernel test robot wrote:
> cppcheck warnings: (new ones prefixed by >>)
> >> mm/sparse.c:145:2: warning: int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn]
>     return (nid << SECTION_NID_SHIFT);

Hrm.

include/linux/mmzone.h:#define SECTION_NID_SHIFT                3

I'm going to suggest that we don't allow for 2^29 node IDs in a single
system, so this doesn't actually represent a bug that needs to be fixed.
On the other hand, this type of bug continually bites us, and it would
be good to warn about this kind of thing.  So in the spirit of silencing
the warning by doing the promotion that C should have specified in
the first place ...

--- 8< ---

[PATCH] Avoid a warning in sparse memory support

cppcheck warns that we're possibly losing information by shifting an int.
It's a false positive, because we don't allow for a NUMA node ID that
large, but if we ever change SECTION_NID_SHIFT, it could become a problem,
and in any case this is usually a legitimate warning.  Fix it by adding
the necessary cast, which makes the compiler generate the right code.

diff --git a/mm/sparse.c b/mm/sparse.c
index 6326cdf36c4f..f17bd4f7caaa 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -143,7 +143,7 @@ unsigned long __section_nr(struct mem_section *ms)
  */
 static inline unsigned long sparse_encode_early_nid(int nid)
 {
-	return (nid << SECTION_NID_SHIFT);
+	return ((unsigned long)nid << SECTION_NID_SHIFT);
 }
 
 static inline int sparse_early_nid(struct mem_section *section)

  reply	other threads:[~2021-07-12 19:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12 19:19 mm/sparse.c:145:2: warning: int result is returned as long value. If the return value is long to avoid loss of information, then you have loss of information. [truncLongCastReturn] kernel test robot
2021-07-12 19:19 ` kernel test robot
2021-07-12 19:41 ` Matthew Wilcox [this message]
2021-07-12 19:41   ` Matthew Wilcox

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=YOya+aBZFFmC476e@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    --cc=rppt@linux.ibm.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 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.