Re: [iproute PATCH v3 0/6] Big C99 style initializer rework

* Re: [iproute PATCH v3 0/6] Big C99 style initializer rework
       [not found] <d6784d8bb2d84ceab6c1719e32e8fa8e@HQ1WP-EXMB11.corp.brocade.com>
@ 2016-06-27 17:59 ` Stephen Hemminger
  2016-06-27 18:23   ` Phil Sutter
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2016-06-27 17:59 UTC (permalink / raw)
  To: Phil Sutter
  Cc: Daniel Borkmann, David Ahern, Nicolas Dichtel, Julien Floret, netdev

On Thu, 23 Jun 2016 17:34:08 +0000
Phil Sutter <phil@nwl.cc> wrote:

> This is v3 of my C99-style initializer related patch series. The changes
> since v2 are:
> 
> - Flattened embedded struct's initializers:
>   Since the field names are very short, I figured it makes more sense to
>   keep indenting low. Also, the same style is already used in
>   ip/xfrm_policy.c so take that as an example.
> 
> - Moved leftover nlmsg_seq initializing into the common place as well:
>   I was unsure whether this is a good idea at first (due to the
>   increment), but again it's done in ip/xfrm_policy.c as well so should
>   be fine.
> 
> - Added a comma after the last field initializer as suggested by Jakub.
> 
> - Dropped patch 7 since it was NACKed.
> 
> - Eliminated checkpatch non-compliance.
> 
> - Second go at union bpf_attr in tc/tc_bpf.c:
>   I figured that while it is not possible to initialize fields, gcc-3.4.6
>   does not complain when setting the whole union to zero using '= {0}'.
>   So I did this and thereby at least got rid of the memset calls.
> 
> For reference, here's the v2 changelog:
> 
> - Rebased onto current upstream master:
>   My own commit a0a73b298a579 ("tc: m_action: Use C99 style initializers
>   for struct req") contains most of the changes to tc/m_action.c already,
>   so I put the remaining ones into a dedicated patch (the first one here)
>   with a better description.
> 
> - Tested against gcc-3.4.6:
>   This is the oldest gcc version I was able to install locally. It indeed
>   does not like the former changes in tc/tc_bpf.c, so I reverted them.
>   Apart from emitting many warnings, it successfully compiles the
>   sources.
> 
> In the process of compatibility testing, I made a few more changes which
> make sense to have:
> 
> - New patch 5 allows to conveniently override the compiler via command
>   line.
> 
> - New patch 6 eliminates a warning with old gcc but looks valid in
>   general.
> 
> - A warning made me look at ip/tcp_metrics.c and I found a minor code
>   simplification (patch 7).
> 
> Phil Sutter (6):
>   tc: m_action: Improve conversion to C99 style initializers
>   Use C99 style initializers everywhere
>   Replace malloc && memset by calloc
>   No need to initialize rtattr fields before parsing
>   Makefile: Allow to override CC
>   misc/ifstat: simplify unsigned value comparison
> 
>  Makefile           |   4 +-
>  bridge/fdb.c       |  25 ++++++------
>  bridge/link.c      |  14 +++----
>  bridge/mdb.c       |  17 ++++-----
>  bridge/vlan.c      |  17 ++++-----
>  genl/ctrl.c        |  44 +++++++++------------
>  genl/genl.c        |   3 +-
>  ip/ip6tunnel.c     |  10 ++---
>  ip/ipaddress.c     |  33 +++++++---------
>  ip/ipaddrlabel.c   |  21 ++++------
>  ip/iplink.c        |  61 ++++++++++++-----------------
>  ip/iplink_can.c    |   4 +-
>  ip/ipmaddr.c       |  25 ++++--------
>  ip/ipmroute.c      |   8 +---
>  ip/ipneigh.c       |  30 ++++++---------
>  ip/ipnetconf.c     |  10 ++---
>  ip/ipnetns.c       |  39 +++++++++----------
>  ip/ipntable.c      |  25 ++++--------
>  ip/iproute.c       |  78 +++++++++++++------------------------
>  ip/iprule.c        |  22 +++++------
>  ip/iptoken.c       |  19 ++++-----
>  ip/iptunnel.c      |  31 +++++----------
>  ip/ipxfrm.c        |  26 ++++---------
>  ip/link_gre.c      |  18 ++++-----
>  ip/link_gre6.c     |  18 ++++-----
>  ip/link_ip6tnl.c   |  25 +++++-------
>  ip/link_iptnl.c    |  22 +++++------
>  ip/link_vti.c      |  18 ++++-----
>  ip/link_vti6.c     |  18 ++++-----
>  ip/xfrm_policy.c   |  99 +++++++++++++++++++----------------------------
>  ip/xfrm_state.c    | 110 ++++++++++++++++++++++-------------------------------
>  lib/libnetlink.c   |  77 ++++++++++++++-----------------------
>  lib/ll_map.c       |   1 -
>  lib/names.c        |   7 +---
>  misc/arpd.c        |  64 ++++++++++++++-----------------
>  misc/ifstat.c      |   2 +-
>  misc/lnstat.c      |   6 +--
>  misc/lnstat_util.c |   4 +-
>  misc/ss.c          |  37 +++++++-----------
>  tc/e_bpf.c         |   7 +---
>  tc/em_canid.c      |   4 +-
>  tc/em_cmp.c        |   4 +-
>  tc/em_ipset.c      |   4 +-
>  tc/em_meta.c       |   4 +-
>  tc/em_nbyte.c      |   4 +-
>  tc/em_u32.c        |   4 +-
>  tc/f_flow.c        |   3 --
>  tc/f_flower.c      |   3 +-
>  tc/f_fw.c          |   6 +--
>  tc/f_route.c       |   3 --
>  tc/f_rsvp.c        |   6 +--
>  tc/f_u32.c         |  12 ++----
>  tc/m_action.c      |  26 ++++---------
>  tc/m_bpf.c         |   5 +--
>  tc/m_csum.c        |   4 +-
>  tc/m_ematch.c      |   4 +-
>  tc/m_gact.c        |   5 +--
>  tc/m_ife.c         |   5 +--
>  tc/m_ipt.c         |  13 ++-----
>  tc/m_mirred.c      |   7 +---
>  tc/m_nat.c         |   4 +-
>  tc/m_pedit.c       |  11 ++----
>  tc/m_police.c      |   5 +--
>  tc/q_atm.c         |   3 +-
>  tc/q_cbq.c         |  22 +++--------
>  tc/q_choke.c       |   4 +-
>  tc/q_codel.c       |   3 +-
>  tc/q_dsmark.c      |   1 -
>  tc/q_fifo.c        |   4 +-
>  tc/q_fq_codel.c    |   3 +-
>  tc/q_hfsc.c        |  13 ++-----
>  tc/q_htb.c         |  15 +++-----
>  tc/q_netem.c       |  16 +++-----
>  tc/q_red.c         |   4 +-
>  tc/q_sfb.c         |  17 ++++-----
>  tc/q_sfq.c         |   4 +-
>  tc/q_tbf.c         |   4 +-
>  tc/tc.c            |   9 ++---
>  tc/tc_bpf.c        |  58 ++++++++++------------------
>  tc/tc_class.c      |  38 +++++++-----------
>  tc/tc_exec.c       |   6 +--
>  tc/tc_filter.c     |  33 ++++++----------
>  tc/tc_qdisc.c      |  33 ++++++----------
>  tc/tc_stab.c       |   4 +-
>  tc/tc_util.c       |   3 +-
>  85 files changed, 565 insertions(+), 977 deletions(-)

I like the idea and it makes code cleaner. But doing this introduces lots of warnings
and that is not acceptable.
ip
    CC       ip.o
    CC       ipaddress.o
ipaddress.c: In function ‘print_queuelen’:
ipaddress.c:175:10: warning: missing braces around initializer [-Wmissing-braces]
   struct ifreq ifr = { 0 };
          ^
ipaddress.c:175:10: warning: (near initialization for ‘ifr.ifr_ifrn’) [-Wmissing-braces]
    CC       ipaddrlabel.o
    CC       iproute.o
    CC       iprule.o
    CC       ipnetns.o
    CC       rtm_map.o
    CC       iptunnel.o
iptunnel.c: In function ‘parse_args’:
iptunnel.c:183:12: warning: missing braces around initializer [-Wmissing-braces]
     struct ip_tunnel_parm old_p = { 0 };
            ^
iptunnel.c:183:12: warning: (near initialization for ‘old_p.name’) [-Wmissing-braces]
iptunnel.c: In function ‘print_tunnel’:
iptunnel.c:296:9: warning: missing braces around initializer [-Wmissing-braces]
  struct ip_tunnel_6rd ip6rd = { 0 };
         ^
iptunnel.c:296:9: warning: (near initialization for ‘ip6rd.prefix’) [-Wmissing-braces]
iptunnel.c:310:10: warning: missing braces around initializer [-Wmissing-braces]
   struct ip_tunnel_prl prl[16] = { 0 };
          ^
iptunnel.c:310:10: warning: (near initialization for ‘prl[0]’) [-Wmissing-braces]
iptunnel.c: In function ‘do_tunnels_list’:
iptunnel.c:402:10: warning: missing braces around initializer [-Wmissing-braces]
   struct ip_tunnel_parm p1 = { 0 };

^ permalink raw reply	[flat|nested] 12+ messages in thread