* check_signed.c false negative
@ 2019-11-11 23:21 John Levon
2019-11-13 17:39 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: John Levon @ 2019-11-11 23:21 UTC (permalink / raw)
To: smatch
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: check_signed.c false negative
2019-11-11 23:21 check_signed.c false negative John Levon
@ 2019-11-13 17:39 ` Dan Carpenter
2019-11-13 17:52 ` John Levon
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-11-13 17:39 UTC (permalink / raw)
To: John Levon; +Cc: smatch
On Mon, Nov 11, 2019 at 11:21:15PM +0000, John Levon wrote:
>
> 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?
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: check_signed.c false negative
2019-11-13 17:39 ` Dan Carpenter
@ 2019-11-13 17:52 ` John Levon
0 siblings, 0 replies; 3+ messages in thread
From: John Levon @ 2019-11-13 17:52 UTC (permalink / raw)
To: Dan Carpenter; +Cc: smatch
On Wed, Nov 13, 2019 at 08:39:13PM +0300, Dan Carpenter wrote:
> On Mon, Nov 11, 2019 at 11:21:15PM +0000, John Levon wrote:
> >
> > 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?
>
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-13 17:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-11 23:21 check_signed.c false negative John Levon
2019-11-13 17:39 ` Dan Carpenter
2019-11-13 17:52 ` John Levon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.