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. And you're right, I saw at least one example that looked like it was using divide when modulo was intended. It's not trivial to understand the intent where this occurs either. Maybe that's what you mean by "mind bending". I'm glad to have helped identify this case. I hope it leads to a few things getting fixed (or improving). -Alex
Anyway, attached. regards, dan carpenter
