From: Stephen Hemminger <hidden> Date: 2016-06-27 17:59:26
On Thu, 23 Jun 2016 17:34:08 +0000
Phil Sutter [off-list ref] 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 };
From: Phil Sutter <phil@nwl.cc> Date: 2016-06-27 18:23:04
Hi,
On Mon, Jun 27, 2016 at 10:59:12AM -0700, Stephen Hemminger wrote:
On Thu, 23 Jun 2016 17:34:08 +0000
Phil Sutter [off-list ref] wrote:
quoted
This is v3 of my C99-style initializer related patch series. The changes
since v2 are:
[...]
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 };
^
From: Stephen Hemminger <hidden> Date: 2016-06-27 21:10:56
On Mon, 27 Jun 2016 20:23:02 +0200
Phil Sutter [off-list ref] wrote:
Hi,
On Mon, Jun 27, 2016 at 10:59:12AM -0700, Stephen Hemminger wrote:
quoted
On Thu, 23 Jun 2016 17:34:08 +0000
Phil Sutter [off-list ref] wrote:
quoted
This is v3 of my C99-style initializer related patch series. The changes
since v2 are:
[...]
quoted
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 };
^
From: Phil Sutter <phil@nwl.cc> Date: 2016-06-28 17:35:49
On Mon, Jun 27, 2016 at 02:10:49PM -0700, Stephen Hemminger wrote:
On Mon, 27 Jun 2016 20:23:02 +0200
Phil Sutter [off-list ref] wrote:
quoted
Hi,
On Mon, Jun 27, 2016 at 10:59:12AM -0700, Stephen Hemminger wrote:
quoted
On Thu, 23 Jun 2016 17:34:08 +0000
Phil Sutter [off-list ref] wrote:
quoted
This is v3 of my C99-style initializer related patch series. The changes
since v2 are:
[...]
quoted
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 };
^
Hmm. In a fresh install of Debian 8.5 I see the warnings as well, but it
has gcc-4.9.2-10 as most recent version.
Another thing I noticed: Using empty braces ('{}') instead of the
universal zero initializer seems to work without causing warnings (at
least unless '-pedantic' is used).
Cheers, Phil
Hmm. In a fresh install of Debian 8.5 I see the warnings as well, but it
has gcc-4.9.2-10 as most recent version.
Another thing I noticed: Using empty braces ('{}') instead of the
universal zero initializer seems to work without causing warnings (at
least unless '-pedantic' is used).
since .ifr_qlen is already referenced in that function seems like your
suggestion above (struct ifreq ifr = { .ifr_qlen = 0 };) should be
acceptable.
Hmm. In a fresh install of Debian 8.5 I see the warnings as well, but it
has gcc-4.9.2-10 as most recent version.
Another thing I noticed: Using empty braces ('{}') instead of the
universal zero initializer seems to work without causing warnings (at
least unless '-pedantic' is used).
since .ifr_qlen is already referenced in that function seems like your
suggestion above (struct ifreq ifr = { .ifr_qlen = 0 };) should be
acceptable.
You mean regarding compatibility of using that define? Or are you
concerned with gcc creating suboptimal code?
I'd rather use a more generic approach than the above. Retrospectively,
I'd rather have that brace orgy instead of the above since it's
intention is more clear and it can be dropped once either gcc guys
manage to backport their fix or the last distribution has updated it's
compiler.
Cheers, Phil
From: David Ahern <hidden> Date: 2016-06-28 17:59:05
On 6/28/16 11:58 AM, Phil Sutter wrote:
quoted
since .ifr_qlen is already referenced in that function seems like your
suggestion above (struct ifreq ifr = { .ifr_qlen = 0 };) should be
acceptable.
You mean regarding compatibility of using that define? Or are you
concerned with gcc creating suboptimal code?
no, I was thinking in terms of open coding knowledge of a struct.
I'd rather use a more generic approach than the above. Retrospectively,
I'd rather have that brace orgy instead of the above since it's
intention is more clear and it can be dropped once either gcc guys
manage to backport their fix or the last distribution has updated it's
compiler.
From: Phil Sutter <phil@nwl.cc> Date: 2016-06-28 18:06:57
On Tue, Jun 28, 2016 at 11:59:04AM -0600, David Ahern wrote:
On 6/28/16 11:58 AM, Phil Sutter wrote:
quoted
quoted
since .ifr_qlen is already referenced in that function seems like your
suggestion above (struct ifreq ifr = { .ifr_qlen = 0 };) should be
acceptable.
You mean regarding compatibility of using that define? Or are you
concerned with gcc creating suboptimal code?
no, I was thinking in terms of open coding knowledge of a struct.
Still not sure if I understand you correctly. These are not typedefs, so
users are supposed to know the internals and removing a field means
potentially breaking every single user.
quoted
I'd rather use a more generic approach than the above. Retrospectively,
I'd rather have that brace orgy instead of the above since it's
intention is more clear and it can be dropped once either gcc guys
manage to backport their fix or the last distribution has updated it's
compiler.
ha, that's funny.
At least someone can laugh about it. :)
Cheers, Phil