* (bug report) b43: impossible conditions in debugfs
@ 2015-11-26 11:59 Dan Carpenter
2015-11-26 12:32 ` Michael Büsch
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2015-11-26 11:59 UTC (permalink / raw)
To: mb; +Cc: linux-wireless, b43-dev
Hello Michael Buesch,
The patch 6bbc321a96d4: "b43: Add debugfs files for random SHM
access" from Jun 19, 2008, leads to the following static checker
warning:
drivers/net/wireless/broadcom/b43/debugfs.c:217 shm32write__write_file()
warn: impossible condition '(mask > 4294967295) => (0-u32max > u32max)'
drivers/net/wireless/broadcom/b43/debugfs.c
198 static int shm32write__write_file(struct b43_wldev *dev,
199 const char *buf, size_t count)
200 {
201 unsigned int routing, addr, mask, set;
202 u32 val;
203 int res;
204
205 res = sscanf(buf, "0x%X 0x%X 0x%X 0x%X",
206 &routing, &addr, &mask, &set);
207 if (res != 4)
208 return -EINVAL;
209 if (routing > B43_MAX_SHM_ROUTING)
210 return -EADDRNOTAVAIL;
211 if (addr > B43_MAX_SHM_ADDR)
212 return -EADDRNOTAVAIL;
213 if (routing == B43_SHM_SHARED) {
214 if ((addr % 2) != 0)
215 return -EADDRNOTAVAIL;
216 }
217 if ((mask > 0xFFFFFFFF) || (set > 0xFFFFFFFF))
Both of these conditions are impossible.
218 return -E2BIG;
219
220 if (mask == 0)
221 val = 0;
222 else
223 val = b43_shm_read32(dev, routing, addr);
224 val &= mask;
225 val |= set;
226 b43_shm_write32(dev, routing, addr, val);
227
228 return 0;
229 }
See also:
drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(mask > 4294967295) => (0-u32max > u32max)'
drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(set > 4294967295) => (0-u32max > u32max)'
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* (bug report) b43: impossible conditions in debugfs
2015-11-26 11:59 (bug report) b43: impossible conditions in debugfs Dan Carpenter
@ 2015-11-26 12:32 ` Michael Büsch
2015-11-26 13:00 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Michael Büsch @ 2015-11-26 12:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-wireless, b43-dev
On Thu, 26 Nov 2015 14:59:24 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Michael Buesch,
>
> The patch 6bbc321a96d4: "b43: Add debugfs files for random SHM
> access" from Jun 19, 2008, leads to the following static checker
> warning:
>
> drivers/net/wireless/broadcom/b43/debugfs.c:217 shm32write__write_file()
> warn: impossible condition '(mask > 4294967295) => (0-u32max > u32max)'
>
> drivers/net/wireless/broadcom/b43/debugfs.c
> 198 static int shm32write__write_file(struct b43_wldev *dev,
> 199 const char *buf, size_t count)
> 200 {
> 201 unsigned int routing, addr, mask, set;
> 202 u32 val;
> 203 int res;
> 204
> 205 res = sscanf(buf, "0x%X 0x%X 0x%X 0x%X",
> 206 &routing, &addr, &mask, &set);
> 207 if (res != 4)
> 208 return -EINVAL;
> 209 if (routing > B43_MAX_SHM_ROUTING)
> 210 return -EADDRNOTAVAIL;
> 211 if (addr > B43_MAX_SHM_ADDR)
> 212 return -EADDRNOTAVAIL;
> 213 if (routing == B43_SHM_SHARED) {
> 214 if ((addr % 2) != 0)
> 215 return -EADDRNOTAVAIL;
> 216 }
> 217 if ((mask > 0xFFFFFFFF) || (set > 0xFFFFFFFF))
>
> Both of these conditions are impossible.
>
> 218 return -E2BIG;
> 219
> 220 if (mask == 0)
> 221 val = 0;
> 222 else
> 223 val = b43_shm_read32(dev, routing, addr);
> 224 val &= mask;
> 225 val |= set;
> 226 b43_shm_write32(dev, routing, addr, val);
> 227
> 228 return 0;
> 229 }
>
> See also:
> drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(mask > 4294967295) => (0-u32max > u32max)'
> drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(set > 4294967295) => (0-u32max > u32max)'
Sure. These are intentional.
The compiler will optimize this out.
--
Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/b43-dev/attachments/20151126/99514e38/attachment.sig>
^ permalink raw reply [flat|nested] 4+ messages in thread
* (bug report) b43: impossible conditions in debugfs
2015-11-26 12:32 ` Michael Büsch
@ 2015-11-26 13:00 ` Dan Carpenter
2015-11-26 13:34 ` Michael Büsch
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2015-11-26 13:00 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, b43-dev
On Thu, Nov 26, 2015 at 01:32:41PM +0100, Michael B?sch wrote:
> > See also:
> > drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(mask > 4294967295) => (0-u32max > u32max)'
> > drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(set > 4294967295) => (0-u32max > u32max)'
>
>
>
> Sure. These are intentional.
> The compiler will optimize this out.
Hm... We try to ignore when people do intentional comparisons with zero
like this:
if (unsigned_var < 0 || unsigned_var >= 10)
return -EINVAL;
Because they are obviously harmless and they don't hurt readability.
Also Linus doesn't like removing these.
But what's the point of this? I have seen these before when I was
checking ide_set_disk_chs() for underflows. I also reported another one
recently that was exactly the same as this and the guy changed it from
if (x > UINT_MAX) to if (!(x < UINT_MAX))... I don't get it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* (bug report) b43: impossible conditions in debugfs
2015-11-26 13:00 ` Dan Carpenter
@ 2015-11-26 13:34 ` Michael Büsch
0 siblings, 0 replies; 4+ messages in thread
From: Michael Büsch @ 2015-11-26 13:34 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-wireless, b43-dev
On Thu, 26 Nov 2015 16:00:34 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Thu, Nov 26, 2015 at 01:32:41PM +0100, Michael B?sch wrote:
> > > See also:
> > > drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(mask > 4294967295) => (0-u32max > u32max)'
> > > drivers/net/wireless/broadcom/b43/debugfs.c:346 mmio32write__write_file() warn: impossible condition '(set > 4294967295) => (0-u32max > u32max)'
> >
> >
> >
> > Sure. These are intentional.
> > The compiler will optimize this out.
>
> Hm... We try to ignore when people do intentional comparisons with zero
> like this:
>
> if (unsigned_var < 0 || unsigned_var >= 10)
> return -EINVAL;
>
> Because they are obviously harmless and they don't hurt readability.
> Also Linus doesn't like removing these.
It just checks whether the value will fit into a 32 bit unsigned int
variable. It doesn't make assumptions on what sizeof(unsigned int) is,
although it would be safe to assume 4 here and omit the check.
--
Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/b43-dev/attachments/20151126/51883ec1/attachment.sig>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-26 13:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-26 11:59 (bug report) b43: impossible conditions in debugfs Dan Carpenter
2015-11-26 12:32 ` Michael Büsch
2015-11-26 13:00 ` Dan Carpenter
2015-11-26 13:34 ` Michael Büsch
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).