search

ClangBuiltLinux / linux

Linux kernel source tree
Other
241 stars 14 forks source link

-Wsometimes-uninitialized in drivers/net/ethernet/stmicro/stmmac/stmmac_main.c #384

Closed nickdesaulniers closed 5 years ago

nickdesaulniers commented 5 years ago

from #381 

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:495:3: error: variable 'ns' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:495:3: error: variable 'ns' is used uninitialized whenever '&&' condition is false [-Werror,-Wsometimes-uninitialized]
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:532:3: error: variable 'ns' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:532:3: error: variable 'ns' is used uninitialized whenever '&&' condition is false [-Werror,-Wsometimes-uninitialized]
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:741:3: error: variable 'sec_inc' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:741:3: error: variable 'sec_inc' is used uninitialized whenever '&&' condition is false [-Werror,-Wsometimes-uninitialized]
nickdesaulniers commented 5 years ago

Seems like the use of stmmac_get_timestamp is problematic in the first 4 cases, and stmmac_config_sub_second_increment in the second. Also, line numbers of the above are off due to them coming from linux-next, but I still see the issue in mainline.

stmmac_get_timestamp expands to stmmac_do_void_callback, which invokes a function it's passed (get_timestamp) and forwards the rest of the args to that function.

The issue is that the check in stmmac_do_void_callback if ever false would not call the callback, and potentially not initialize the variable.

Doesn't hurt to just initialize the variable ns.

sec_inc has the same problem in stmmac_config_sub_second_increment which expands to stmmac_do_void_callback again but invokes the passed callback config_sub_second_increment which may or may not get called. Almost every other local variable in stmmac_config_sub_second_increment is zero initialized, except sec_inc.

nathanchance commented 5 years ago

Patch sent: https://lore.kernel.org/lkml/20190307162101.29204-1-natechancellor@gmail.com/

nathanchance commented 5 years ago

Patch accepted: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=df103170854e87124ee7bdd2bca64b178e653f97

nathanchance commented 5 years ago

Another fix accepted: https://git.kernel.org/davem/net/c/1f5d861f7fefa971b2c6e766f77932c86419a319

nathanchance commented 5 years ago

Merged into mainline:

https://git.kernel.org/torvalds/c/df103170854e87124ee7bdd2bca64b178e653f97

https://git.kernel.org/torvalds/c/1f5d861f7fefa971b2c6e766f77932c86419a319