Re: [patch] ipmi: make static checkers happy about range checks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I'm ok with this, I don't think it will make any real difference besides the compiler converting that comparison to unsigned. I wish I had made that unsigned.

-corey

On 09/17/2010 02:26 PM, Dan Carpenter wrote:
I'm working on a Smatch check that complains that addr->channel comes
from the user and it might be negative.  It actually can *not* be
negative because we already checked that when we called
ipmi_validate_addr() from handle_send_req().  Also it can't be greater
than or equal to IPMI_MAX_CHANNELS because we already verified that as
well.

Although this change doesn't do anything, it's still a cleanup because
it makes the checking consistent and also it silences the static
analysis warnings.

Signed-off-by: Dan Carpenter<error27@xxxxxxxxx>
---
Really, I would prefer to just make ->channel unsigned but the type
comes from the IPMI spec.

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 4f3f8c9..57f4c80 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -1527,7 +1527,7 @@ static int i_ipmi_request(ipmi_user_t          user,
  		long                  seqid;
  		int                   broadcast = 0;

-		if (addr->channel>= IPMI_MAX_CHANNELS) {
+		if (addr->channel<  0 || addr->channel>= IPMI_MAX_CHANNELS) {
  			ipmi_inc_stat(intf, sent_invalid_commands);
  			rv = -EINVAL;
  			goto out_err;
@@ -1657,7 +1657,7 @@ static int i_ipmi_request(ipmi_user_t          user,
  		unsigned char         ipmb_seq;
  		long                  seqid;

-		if (addr->channel>= IPMI_MAX_CHANNELS) {
+		if (addr->channel<  0 || addr->channel>= IPMI_MAX_CHANNELS) {
  			ipmi_inc_stat(intf, sent_invalid_commands);
  			rv = -EINVAL;
  			goto out_err;
@@ -1797,7 +1797,7 @@ static int check_addr(ipmi_smi_t       intf,
  		      unsigned char    *saddr,
  		      unsigned char    *lun)
  {
-	if (addr->channel>= IPMI_MAX_CHANNELS)
+	if (addr->channel<  0 || addr->channel>= IPMI_MAX_CHANNELS)
  		return -EINVAL;
  	*lun = intf->channels[addr->channel].lun;
  	*saddr = intf->channels[addr->channel].address;

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux