Messages in this thread | | | From | Nick Desaulniers <> | Date | Wed, 20 Mar 2019 13:50:59 -0700 | Subject | Re: -Wsometimes-uninitialized Clang warning in net/tipc/node.c |
| |
On Wed, Mar 20, 2019 at 12:07 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > On Thu, Mar 07, 2019 at 05:17:23PM -0700, Nathan Chancellor wrote: > > Hi all, > > > > We are trying to get Clang's -Wsometimes-uninitialized turned on for the > > kernel as it can catch some bugs that GCC can't. This warning came up: > > > > net/tipc/node.c:831:6: warning: variable 'maddr' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] > > if (!tipc_link_is_establishing(l)) { > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > net/tipc/node.c:847:46: note: uninitialized use occurs here > > tipc_bearer_xmit(n->net, bearer_id, &xmitq, maddr); > > ^~~~~ > > net/tipc/node.c:831:2: note: remove the 'if' if its condition is always true > > if (!tipc_link_is_establishing(l)) { > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > net/tipc/node.c:821:31: note: initialize the variable 'maddr' to silence this warning > > struct tipc_media_addr *maddr; > > ^ > > = NULL > > 1 warning generated. > > > > This definitely appears to be a legitimate warning but I'm not sure of > > the proper solution (should maddr be initialized to NULL or should it be > > set to something different in the else branch). Your input would be > > greatly appreciated. > > > > Cheers, > > Nathan > > Gentle ping (if there was a response to this, I didn't receive it). I > know I sent it in the middle of a merge window so I get if it slipped > through the cracks. > > Thanks, > Nathan
The use in tipc_bearer_xmit() isn't even guaranteed to set the in/out parameter, so even if the if is taken doesn't guarantee that maddr is always initialized before calling tipc_bearer_xmit().
At the minimum, we should initialize maddr to NULL. I think we'd prefer to risk the possibility of a null pointer dereference to the possibility of working with uninitialized memory. To be clear, both are bad, but one is easier to spot/debug later than the other.
Thanks for bringing this up for discussion, sorry I missed it before. -- Thanks, ~Nick Desaulniers
| |