From: Brian Norris <computersforpeace@gmail.com>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH][thermal-next] thermal: brcmstb: remove two redundant integer range checks
Date: Wed, 06 Sep 2017 18:40:46 +0000 [thread overview]
Message-ID: <20170906184046.GB35422@google.com> (raw)
In-Reply-To: <20170903131619.6857-1-colin.king@canonical.com>
Hi,
On Sun, Sep 03, 2017 at 02:16:19PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The comparisons for integer low on low > INT_MAX and also
> integer high > INT_MAX are never going to be true since an
> int type cannot be greater than INT_MAX. Remove these redundant
> checks.
>
> Detected by: CoverityScan CID#1455245, 1455248 ("Operands don't affect
> result (CONSTANT_EXPRESSION_RESULT)")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/thermal/broadcom/brcmstb_thermal.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> index 87b8e7a86ee3..bcb3945feea6 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -278,8 +278,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
>
> if (low) {
> - if (low > INT_MAX)
> - low = INT_MAX;
> avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
> avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
> } else {
> @@ -287,8 +285,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> }
>
> if (high < ULONG_MAX) {
Dan's robots noticed that the above condition is not useful either (on
architectures where INT_MAX < ULONG_MAX -- i.e., all?), since 'high' is
'int', not 'unsigned long'.
Should probably fix that
s/ULONG_MAX/INT_MAX/
?
Brian
> - if (high > INT_MAX)
> - high = INT_MAX;
> avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
> avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
> } else {
> --
> 2.14.1
>
WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <computersforpeace@gmail.com>
To: Colin King <colin.king@canonical.com>
Cc: Markus Mayer <mmayer@broadcom.com>,
bcm-kernel-feedback-list@broadcom.com,
Zhang Rui <rui.zhang@intel.com>,
Eduardo Valentin <edubezval@gmail.com>,
Gregory Fong <gregory.0xf0@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH][thermal-next] thermal: brcmstb: remove two redundant integer range checks
Date: Wed, 6 Sep 2017 11:40:46 -0700 [thread overview]
Message-ID: <20170906184046.GB35422@google.com> (raw)
In-Reply-To: <20170903131619.6857-1-colin.king@canonical.com>
Hi,
On Sun, Sep 03, 2017 at 02:16:19PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The comparisons for integer low on low > INT_MAX and also
> integer high > INT_MAX are never going to be true since an
> int type cannot be greater than INT_MAX. Remove these redundant
> checks.
>
> Detected by: CoverityScan CID#1455245, 1455248 ("Operands don't affect
> result (CONSTANT_EXPRESSION_RESULT)")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/thermal/broadcom/brcmstb_thermal.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> index 87b8e7a86ee3..bcb3945feea6 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -278,8 +278,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
>
> if (low) {
> - if (low > INT_MAX)
> - low = INT_MAX;
> avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
> avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
> } else {
> @@ -287,8 +285,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> }
>
> if (high < ULONG_MAX) {
Dan's robots noticed that the above condition is not useful either (on
architectures where INT_MAX < ULONG_MAX -- i.e., all?), since 'high' is
'int', not 'unsigned long'.
Should probably fix that
s/ULONG_MAX/INT_MAX/
?
Brian
> - if (high > INT_MAX)
> - high = INT_MAX;
> avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
> avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
> } else {
> --
> 2.14.1
>
WARNING: multiple messages have this Message-ID (diff)
From: computersforpeace@gmail.com (Brian Norris)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH][thermal-next] thermal: brcmstb: remove two redundant integer range checks
Date: Wed, 6 Sep 2017 11:40:46 -0700 [thread overview]
Message-ID: <20170906184046.GB35422@google.com> (raw)
In-Reply-To: <20170903131619.6857-1-colin.king@canonical.com>
Hi,
On Sun, Sep 03, 2017 at 02:16:19PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The comparisons for integer low on low > INT_MAX and also
> integer high > INT_MAX are never going to be true since an
> int type cannot be greater than INT_MAX. Remove these redundant
> checks.
>
> Detected by: CoverityScan CID#1455245, 1455248 ("Operands don't affect
> result (CONSTANT_EXPRESSION_RESULT)")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/thermal/broadcom/brcmstb_thermal.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c
> index 87b8e7a86ee3..bcb3945feea6 100644
> --- a/drivers/thermal/broadcom/brcmstb_thermal.c
> +++ b/drivers/thermal/broadcom/brcmstb_thermal.c
> @@ -278,8 +278,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> dev_dbg(priv->dev, "set trips %d <--> %d\n", low, high);
>
> if (low) {
> - if (low > INT_MAX)
> - low = INT_MAX;
> avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_LOW, low);
> avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_LOW, 1);
> } else {
> @@ -287,8 +285,6 @@ static int brcmstb_set_trips(void *data, int low, int high)
> }
>
> if (high < ULONG_MAX) {
Dan's robots noticed that the above condition is not useful either (on
architectures where INT_MAX < ULONG_MAX -- i.e., all?), since 'high' is
'int', not 'unsigned long'.
Should probably fix that
s/ULONG_MAX/INT_MAX/
?
Brian
> - if (high > INT_MAX)
> - high = INT_MAX;
> avs_tmon_set_trip_temp(priv, TMON_TRIP_TYPE_HIGH, high);
> avs_tmon_trip_enable(priv, TMON_TRIP_TYPE_HIGH, 1);
> } else {
> --
> 2.14.1
>
next prev parent reply other threads:[~2017-09-06 18:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-03 13:16 [PATCH][thermal-next] thermal: brcmstb: remove two redundant integer range checks Colin King
2017-09-03 13:16 ` Colin King
2017-09-03 13:16 ` Colin King
2017-09-06 18:40 ` Brian Norris [this message]
2017-09-06 18:40 ` Brian Norris
2017-09-06 18:40 ` Brian Norris
2017-09-08 2:59 ` Eduardo Valentin
2017-09-08 2:59 ` Eduardo Valentin
2017-09-08 2:59 ` Eduardo Valentin
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=20170906184046.GB35422@google.com \
--to=computersforpeace@gmail.com \
--cc=linux-arm-kernel@lists.infradead.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.