On Sat, Nov 19, 2022 at 03:21:48AM -0600, Alex Elder wrote:
> On 11/18/22 03:50, Dan Carpenter wrote:
> > On Thu, Nov 17, 2022 at 07:47:27AM +0300, Dan Carpenter wrote:
> > > Heh. It really feels like this line should have generated a checker
> > > warning as well. I've created two versions. The first complains when
> > > ever there is a divide used as a condition:
> > >
> > > if (a / b) {
> > >
> > > The other complains when it's part of a logical && or ||.
> > >
> > > if (a && a / b) {
> > >
> > > drivers/net/ipa/ipa_table.c:414 ipa_table_init_add() warn: divide condition: 'hash_mem->size / 8'
> > > drivers/net/ipa/ipa_table.c:414 ipa_table_init_add() warn: divide condition (logical): 'hash_mem->size / 8'
> > >
> > > I'll test them out tonight and see if either gives useful results.
> >
> > I modified the test to ignore macros. Otherwise we get warnings where
> > macros are trying to avoid divide by zero bugs. There was no advantage
> > in treating logicals as special so I dropped that.
> >
> > The results are kind of mind bending. I think maybe people sometimes
> > accidentally write "if (a / b) {" meaning does it divide cleanly? When
> > that should be written as: "if ((a % b) == 0) {".
>
> Interesting. I looked at the first few. I think the nvdimm ones
> might be using "if (cleared / 512)" to mean "if (cleared >= 512",
> and in that case, the >= might actually be more efficient than the
> divide. On the real-time clock one it looked like a similar usage.
> Regardless, it's not a typical idiom so I don't think it's
> straightforward to understand.
Yeah. I'm going to publish the check and the new warning message will
be:
drivers/nvdimm/claim.c:287 nsio_rw_bytes() warn: replace divide condition 'cleared / 512' with 'cleared >= 512'
regards,
dan carpenter
