| Message ID | 20240516193700.212737-6-mkp@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | clang: Fix Clang's static analyzer detections. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Bleep bloop. Greetings Mike Pattrick, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject, '<area>: <summary>', is over 70 characters, i.e., 72. Subject: netdev-linux: Fix Clang's static analyzer uninitialized values warnings. Lines checked: 53, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On 5/16/24 21:37, Mike Pattrick wrote: > Clang's static analyzer will complain about an uninitialized value > because in some error conditions we wouldn't set all values that are > used later. Some more details would be nice here. > > Now we initialize more values that are needed later even in error > conditions. > > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- Same for the subject line and a Fixes tag. Also, there seems to be two separate issues fixed here, they should be separate patches with distinct names and Fixes tags as they may need to go to different branches (I didn't check). > lib/netdev-linux.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 25349c605..66dae3e1a 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -2439,7 +2439,9 @@ netdev_linux_read_definitions(struct netdev_linux *netdev, > int error = 0; > > error = netdev_linux_read_stringset_info(netdev, &len); > - if (error || !len) { > + if (!len) { > + return -EOPNOTSUPP; > + } else if (error) { > return error; > } > strings = xzalloc(sizeof *strings + len * ETH_GSTRING_LEN); > @@ -2724,6 +2726,7 @@ netdev_linux_get_speed_locked(struct netdev_linux *netdev, > uint32_t *current, uint32_t *max) > { > if (netdev_linux_netnsid_is_remote(netdev)) { > + *current = 0; > return EOPNOTSUPP; > } > > @@ -2733,6 +2736,8 @@ netdev_linux_get_speed_locked(struct netdev_linux *netdev, > ? 0 : netdev->current_speed; > *max = MIN(UINT32_MAX, > netdev_features_to_bps(netdev->supported, 0) / 1000000ULL); > + } else { > + *current = 0; > } > return netdev->get_features_error; > } We should be clearing both the current and a max for consistency. Best regards, Ilya Maximets.
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 25349c605..66dae3e1a 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2439,7 +2439,9 @@ netdev_linux_read_definitions(struct netdev_linux *netdev, int error = 0; error = netdev_linux_read_stringset_info(netdev, &len); - if (error || !len) { + if (!len) { + return -EOPNOTSUPP; + } else if (error) { return error; } strings = xzalloc(sizeof *strings + len * ETH_GSTRING_LEN); @@ -2724,6 +2726,7 @@ netdev_linux_get_speed_locked(struct netdev_linux *netdev, uint32_t *current, uint32_t *max) { if (netdev_linux_netnsid_is_remote(netdev)) { + *current = 0; return EOPNOTSUPP; } @@ -2733,6 +2736,8 @@ netdev_linux_get_speed_locked(struct netdev_linux *netdev, ? 0 : netdev->current_speed; *max = MIN(UINT32_MAX, netdev_features_to_bps(netdev->supported, 0) / 1000000ULL); + } else { + *current = 0; } return netdev->get_features_error; }
Clang's static analyzer will complain about an uninitialized value because in some error conditions we wouldn't set all values that are used later. Now we initialize more values that are needed later even in error conditions. Signed-off-by: Mike Pattrick <mkp@redhat.com> --- lib/netdev-linux.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)