check_signed.c false negative
Given:
enum ib_qp_state {
IB_QPS_RESET,
IB_QPS_INIT,
IB_QPS_RTR,
IB_QPS_RTS,
IB_QPS_SQD,
IB_QPS_SQE,
IB_QPS_ERR
};int p(enum ib_qp_state cur_state, enum ib_qp_state next_state) {
#if 0
if (cur_state < 0 || cur_state > IB_QPS_ERR ||
next_state < 0 || next_state > IB_QPS_ERR) {
return (5);
}
#else
if (next_state < 0 || next_state > IB_QPS_ERR) {
return (5);
}#endif
return (0); }
current smatch is happy. But "#if 1" above instead, and:
jlevon@sent:~/src/smatch$ ./smatch a.c ./smatch: a.c:15 p() warn: unsigned 'cur_state' is never less than zero.
Why is "next_state" not reported equally?
thanks
john
Quoted 29 lines · John Levon
Given:
enum ib_qp_state { IB_QPS_RESET, IB_QPS_INIT, IB_QPS_RTR, IB_QPS_RTS, IB_QPS_SQD, IB_QPS_SQE, IB_QPS_ERR };int
p(enum ib_qp_state cur_state, enum ib_qp_state next_state){#if 0 if (cur_state < 0 || cur_state > IB_QPS_ERR || next_state < 0 || next_state > IB_QPS_ERR) { return (5); } #else if (next_state < 0 || next_state > IB_QPS_ERR) { return (5); }#endif
return (0); }
Quoted 6 lines
current smatch is happy. But "#if 1" above instead, and:
jlevon@sent:~/src/smatch$ ./smatch a.c ./smatch: a.c:15 p() warn: unsigned 'cur_state' is never less than zero.Why is "
next_state" not reported equally?
My plan was that neither one would print a warning. When people do
if (foo < 0 || foo > max)
then normally it's not a real life bug... Linus said at kernel summit that he doesn't like when people send patches for those.
The problem is that when I wrote the code to silence the warning then I hadn't created all the expr_get_parent_expr() function so I couldn't silence it properly.
regards,
dan carpenter
Quoted 35 lines · Dan Carpenter
Quoted 34 lines · John Levon
Given:
enum ib_qp_state { IB_QPS_RESET, IB_QPS_INIT, IB_QPS_RTR, IB_QPS_RTS, IB_QPS_SQD, IB_QPS_SQE, IB_QPS_ERR };int
p(enum ib_qp_state cur_state, enum ib_qp_state next_state){#if 0 if (cur_state < 0 || cur_state > IB_QPS_ERR || next_state < 0 || next_state > IB_QPS_ERR) { return (5); } #else if (next_state < 0 || next_state > IB_QPS_ERR) { return (5); }#endif
return (0); }current smatch is happy. But "#if 1" above instead, and:
jlevon@sent:~/src/smatch$ ./smatch a.c ./smatch: a.c:15 p() warn: unsigned 'cur_state' is never less than zero.
Quoted 8 lines
Quoted 1 line
Why is "
next_state" not reported equally?My plan was that neither one would print a warning. When people do
if (foo < 0 || foo > max)then normally it's not a real life bug... Linus said at kernel summit that he doesn't like when people send patches for those.
Right, in fact the code this comes from is similar: it's a belt-and-braces check that we didn't somehow get a bad state value.
There's definitely other cases where this might be useful, though I wouldn't object to disabling it by default altogether.
thanks
john