On 18 Oct 2023, at 12:50, Simon Horman wrote:

> On Tue, Oct 17, 2023 at 03:49:41PM +0200, Eelco Chaudron wrote:
>> This patch fixes two 'Dead assignment' warnings, where the one in
>> count_common_prefix_run() is actually a bug where the set is in reverse 
>> order.
>
> Hi Eelco,
>
> I agree with the correctness of these changes, and that they
> address clang-tidy warnings. But if one is a bug should it be in a
> separate patch, with a fixes tag, and  description of the
> problem that is being addressed?

ACK, makes sense, let me split up the patch. I found the problem when looking 
at the reported issue.

>> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
>> ---
>>  lib/ofp-monitor.c |    5 ++---
>>  lib/ofp-table.c   |    2 +-
>>  2 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/ofp-monitor.c b/lib/ofp-monitor.c
>> index c27733a52..aed5f0497 100644
>> --- a/lib/ofp-monitor.c
>> +++ b/lib/ofp-monitor.c
>> @@ -962,13 +962,12 @@ ofputil_decode_flow_update(struct ofputil_flow_update 
>> *update,
>>              return 0;
>>          } else if (update->event == OFPFME_PAUSED
>>                     || update->event == OFPFME_RESUMED) {
>> -            struct ofp_flow_update_paused *ofup;
>>
>> -            if (length != sizeof *ofup) {
>> +            if (length != sizeof(struct ofp_flow_update_paused)) {
>>                  goto bad_len;
>>              }
>>
>> -            ofup = ofpbuf_pull(msg, sizeof *ofup);
>> +            ofpbuf_pull(msg, sizeof(struct ofp_flow_update_paused));
>>              return 0;
>>          } else if (update->event == OFPFME_INITIAL
>>                     || update->event == OFPFME_ADDED
>> diff --git a/lib/ofp-table.c b/lib/ofp-table.c
>> index a956754f2..f9bd3b7f9 100644
>> --- a/lib/ofp-table.c
>> +++ b/lib/ofp-table.c
>> @@ -1416,7 +1416,7 @@ count_common_prefix_run(const char *ids[], size_t n,
>>          if (!next) {
>>              break;
>>          } else if (next < extra_prefix_len) {
>> -            next = extra_prefix_len;
>> +            extra_prefix_len = next;
>>          }
>>          i++;
>>      }
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to