Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

-Wsometimes-uninitialized in drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c #1390

Closed
nathanchance opened this issue Jun 3, 2021 · 2 comments
Closed

-Wsometimes-uninitialized in drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c #1390

nathanchance opened this issue Jun 3, 2021 · 2 comments
Assignees
@nathanchance
Labels
-Wsometimes-uninitialized [BUG] linux-next This is an issue only seen in linux-next [FIXED][LINUX] development cycle This bug was only present and fixed in a -next or -rc cycle

Comments

@nathanchance
Copy link
Member 

drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:283:14: warning: variable 'proto' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                } else if (skb->protocol == htons(ETH_P_IPV6)) {
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:295:36: note: uninitialized use occurs here
                check = rmnet_map_get_csum_field(proto, trans);
                                                 ^~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:283:10: note: remove the 'if' if its condition is always true
                } else if (skb->protocol == htons(ETH_P_IPV6)) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:270:11: note: initialize the variable 'proto' to silence this warning
                u8 proto;
                        ^
                         = '\0'
1 warning generated.

Patch submitted: https://lore.kernel.org/r/20210603173410.310362-1-nathan@kernel.org/
@nathanchance nathanchance added [PATCH] Submitted A patch has been submitted for review [BUG] linux-next This is an issue only seen in linux-next -Wsometimes-uninitialized labels Jun 3, 2021
@nathanchance nathanchance self-assigned this Jun 3, 2021
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jun 3, 2021
@nathanchance @intel-lab-lkp
net: ethernet: rmnet: Restructure if checks to avoid uninitialized wa…
a998c28 
…rning

Clang warns that proto in rmnet_map_v5_checksum_uplink_packet() might be
used uninitialized:

drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:283:14: warning:
variable 'proto' is used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
                } else if (skb->protocol == htons(ETH_P_IPV6)) {
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:295:36: note:
uninitialized use occurs here
                check = rmnet_map_get_csum_field(proto, trans);
                                                 ^~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:283:10: note:
remove the 'if' if its condition is always true
                } else if (skb->protocol == htons(ETH_P_IPV6)) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:270:11: note:
initialize the variable 'proto' to silence this warning
                u8 proto;
                        ^
                         = '\0'
1 warning generated.

This is technically a false positive because there is an if statement
above this one that checks skb->protocol for not being either
ETH_P_IP{,V6}. However, it is more obvious to sink that into the if
statement as an else branch, which makes the code clearer and fixes the
warning.

At the same time, move the "IS_ENABLED(CONFIG_IPV6)" into the else if
condition so that the else branch of the preprocessor conditional can
be shared, since there is no build failure with CONFIG_IPV6 disabled.

Fixes: b6e5d27 ("net: ethernet: rmnet: Add support for MAPv5 egress packets")
Link: ClangBuiltLinux#1390
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
@nickdesaulniers
Copy link
Member

accepted: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=118de6106735

@nickdesaulniers nickdesaulniers added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Submitted A patch has been submitted for review labels Jun 3, 2021
@nathanchance
Copy link
Member Author

Fixed: https://git.kernel.org/next/linux-next/c/118de6106735

@nathanchance nathanchance added [FIXED][LINUX] development cycle This bug was only present and fixed in a -next or -rc cycle and removed [PATCH] Accepted A submitted patch has been accepted upstream labels Jun 4, 2021
JeevakaPrabu pushed a commit to projectceladon-ci/linux-intel-lts2019-chromium that referenced this issue Aug 5, 2021
@nathanchance
UPSTREAM: net: ethernet: rmnet: Restructure if checks to avoid uninit…
2855823 
…ialized warning

Clang warns that proto in rmnet_map_v5_checksum_uplink_packet() might be
used uninitialized:

drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:283:14: warning:
variable 'proto' is used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
                } else if (skb->protocol == htons(ETH_P_IPV6)) {
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:295:36: note:
uninitialized use occurs here
                check = rmnet_map_get_csum_field(proto, trans);
                                                 ^~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:283:10: note:
remove the 'if' if its condition is always true
                } else if (skb->protocol == htons(ETH_P_IPV6)) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c:270:11: note:
initialize the variable 'proto' to silence this warning
                u8 proto;
                        ^
                         = '\0'
1 warning generated.

This is technically a false positive because there is an if statement
above this one that checks skb->protocol for not being either
ETH_P_IP{,V6}. However, it is more obvious to sink that into the if
statement as an else branch, which makes the code clearer and fixes the
warning.

At the same time, move the "IS_ENABLED(CONFIG_IPV6)" into the else if
condition so that the else branch of the preprocessor conditional can
be shared, since there is no build failure with CONFIG_IPV6 disabled.

Fixes: b6e5d27e32ef ("net: ethernet: rmnet: Add support for MAPv5 egress packets")
Link: ClangBuiltLinux/linux#1390
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 118de6106735cfeb04daf9de1d5a9f953ac034ba)

BUG=b:191413769
TEST=LTE fuctional

Signed-off-by: Sujit Kautkar <sujitka@chromium.org>
Change-Id: I802b0c15135c5b07d641ed3d9cbde392f4fb8e6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2979545
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3003860
Reviewed-by: Daniel Winkler <danielwinkler@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nathanchance nathanchance

Labels
-Wsometimes-uninitialized [BUG] linux-next This is an issue only seen in linux-next [FIXED][LINUX] development cycle This bug was only present and fixed in a -next or -rc cycle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nickdesaulniers @nathanchance