[v2,iproute2-next,00/11] clang warning fixes

Message

Stephen Hemminger Jan. 11, 2022, 5:54 p.m. UTC

This patch set makes iproute2-next main branch compile without warnings
on Clang 11 (and probably later versions).

Still needs more testing before merge. There are likely to be some
unnecessary output format changes from this.

v2
  - use print name value


Stephen Hemminger (11):
  tc: add format attribute to tc_print_rate
  utils: add format attribute
  netem: fix clang warnings
  flower: fix clang warnings
  tc_util: fix clang warning in print_masked_type
  ipl2tp: fix clang warning
  can: fix clang warning
  tipc: fix clang warning about empty format string
  tunnel: fix clang warning
  libbpf: fix clang warning about format non-literal
  json_print: suppress clang format warning

 include/utils.h  |  4 +++-
 ip/ipl2tp.c      |  5 +++--
 ip/iplink_can.c  |  5 +++--
 ip/tunnel.c      |  9 +--------
 lib/bpf_libbpf.c |  6 ++++--
 lib/json_print.c |  8 ++++++++
 tc/f_flower.c    | 38 ++++++++++++--------------------------
 tc/q_netem.c     | 33 ++++++++++++++++++++-------------
 tc/tc_util.c     | 20 +++++++-------------
 tipc/link.c      |  2 +-
 10 files changed, 62 insertions(+), 68 deletions(-)

Comments

David Ahern Jan. 16, 2022, 11:18 p.m. UTC | #1

On 1/11/22 10:54 AM, Stephen Hemminger wrote:
> This patch set makes iproute2-next main branch compile without warnings
> on Clang 11 (and probably later versions).
> 
> Still needs more testing before merge. There are likely to be some
> unnecessary output format changes from this.
> 

I think the tc patches are the only likely candidates. The
print_string_name_value conversion should be clean.

Jamal: As I recall you have a test suite for tc. Can you test this set?

Stephen Hemminger Jan. 17, 2022, 12:33 a.m. UTC | #2

On Sun, 16 Jan 2022 16:18:53 -0700
David Ahern <[e-mail]> wrote:

> On 1/11/22 10:54 AM, Stephen Hemminger wrote:
> > This patch set makes iproute2-next main branch compile without warnings
> > on Clang 11 (and probably later versions).
> > 
> > Still needs more testing before merge. There are likely to be some
> > unnecessary output format changes from this.
> >   
> 
> I think the tc patches are the only likely candidates. The
> print_string_name_value conversion should be clean.
> 
> Jamal: As I recall you have a test suite for tc. Can you test this set?

There was a blank after newline and with print_string_name_value that goes away.
Lets introduce something like print_nl_indent() to handle that

Jamal Hadi Salim Jan. 17, 2022, 3:29 p.m. UTC | #3

On 2022-01-16 18:18, David Ahern wrote:
> On 1/11/22 10:54 AM, Stephen Hemminger wrote:
>> This patch set makes iproute2-next main branch compile without warnings
>> on Clang 11 (and probably later versions).
>>
>> Still needs more testing before merge. There are likely to be some
>> unnecessary output format changes from this.
>>
> 
> I think the tc patches are the only likely candidates. The
> print_string_name_value conversion should be clean.
> 
> Jamal: As I recall you have a test suite for tc. Can you test this set?

We try to push, whenever we can, to kernel tdc tests. The Intel robot
should catch issues based on what we have there. If we make part of the
acceptance process (incumbent on people who create the patches) to
run those tests it would help getting cleaner submissions. Not sure
if we can have a bot doing this..

Punting to Victor(on Cc) to run the tests and double check if we
have test  cases that cover for these changes.

cheers,
jamal

Victor Nogueira Jan. 20, 2022, 12:48 p.m. UTC | #4

Hi,
Sorry for not responding sooner. I patched iproute2 and several
existing tests failed.
Example:
Test 696a: Add simple ct action

All test results:

1..1
not ok 1 696a - Add simple ct action
Could not match regex pattern. Verify command output:
total acts 1

action order 0: ct
zone 0 pipe
index 42 ref 1 bind 0

The problem is the additional new line added.

WIthout this patch:
https://patchwork.kernel.org/project/netdevbpf/patch/[e-mail]/
it the output of tc actions list action ct is:

total acts 1

action order 0: ct zone 0 pipe
index 42 ref 1 bind 0

With it it is:

total acts 1

action order 0: ct
zone 0 pipe
index 42 ref 1 bind 0

So I believe the problem is just formatting, however it still breaks some tests

cheers,
Victor

On 17 Jan 2022, at 12:29, Jamal Hadi Salim <[e-mail]> wrote:

On 2022-01-16 18:18, David Ahern wrote:

On 1/11/22 10:54 AM, Stephen Hemminger wrote:

This patch set makes iproute2-next main branch compile without warnings
on Clang 11 (and probably later versions).

Still needs more testing before merge. There are likely to be some
unnecessary output format changes from this.

I think the tc patches are the only likely candidates. The
print_string_name_value conversion should be clean.
Jamal: As I recall you have a test suite for tc. Can you test this set?


We try to push, whenever we can, to kernel tdc tests. The Intel robot
should catch issues based on what we have there. If we make part of the
acceptance process (incumbent on people who create the patches) to
run those tests it would help getting cleaner submissions. Not sure
if we can have a bot doing this..

Punting to Victor(on Cc) to run the tests and double check if we
have test  cases that cover for these changes.

cheers,
jamal

David Ahern Jan. 20, 2022, 7:20 p.m. UTC | #5

On 1/20/22 5:48 AM, Victor Nogueira wrote:
> Hi,
> Sorry for not responding sooner. I patched iproute2 and several
> existing tests failed.
> Example:
> Test 696a: Add simple ct action
> 
> All test results:
> 
> 1..1
> not ok 1 696a - Add simple ct action
> Could not match regex pattern. Verify command output:
> total acts 1
> 
> action order 0: ct
> zone 0 pipe
> index 42 ref 1 bind 0
> 
> The problem is the additional new line added.
> 
> WIthout this patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/[e-mail]/
> it the output of tc actions list action ct is:
> 
> total acts 1
> 
> action order 0: ct zone 0 pipe
> index 42 ref 1 bind 0
> 
> With it it is:
> 
> total acts 1
> 
> action order 0: ct
> zone 0 pipe
> index 42 ref 1 bind 0
> 
> So I believe the problem is just formatting, however it still breaks some tests
> 

Thanks, Victor. Hopefully that is addressed in v3 of the set.

Victor Nogueira Jan. 20, 2022, 8:21 p.m. UTC | #6

On Thu, Jan 20, 2022 at 4:20 PM David Ahern <[e-mail]> wrote:
>
> On 1/20/22 5:48 AM, Victor Nogueira wrote:
> > Hi,
> > Sorry for not responding sooner. I patched iproute2 and several
> > existing tests failed.
> > Example:
> > Test 696a: Add simple ct action
> >
> > All test results:
> >
> > 1..1
> > not ok 1 696a - Add simple ct action
> > Could not match regex pattern. Verify command output:
> > total acts 1
> >
> > action order 0: ct
> > zone 0 pipe
> > index 42 ref 1 bind 0
> >
> > The problem is the additional new line added.
> >
> > WIthout this patch:
> > https://patchwork.kernel.org/project/netdevbpf/patch/[e-mail]/
> > it the output of tc actions list action ct is:
> >
> > total acts 1
> >
> > action order 0: ct zone 0 pipe
> > index 42 ref 1 bind 0
> >
> > With it it is:
> >
> > total acts 1
> >
> > action order 0: ct
> > zone 0 pipe
> > index 42 ref 1 bind 0
> >
> > So I believe the problem is just formatting, however it still breaks some tests
> >
>
> Thanks, Victor. Hopefully that is addressed in v3 of the set.
>

Actually, I tested using v3 patches.
Sorry, I should've said that in the previous email.

cheers,
Victor