Date: Mon, 14 Apr 2014 17:35:32 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Christian Brueffer <brueffer@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r264421 - head/sys/netpfil/ipfw Message-ID: <20140414170654.Y5260@besplex.bde.org> In-Reply-To: <201404132113.s3DLDXRI044236@svn.freebsd.org> References: <201404132113.s3DLDXRI044236@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 13 Apr 2014, Christian Brueffer wrote:
> Log:
> Free resources and error cases; re-indent a curly brace while here.
The patch begins with a misindented block in old code. I thought at
first the whole file was misindented with 4-column indents and the
patch is backwards, as in a recent commit to ata, but here it is mainly
the is7 code that is misindented, while in ata the indentation was
already mangled throughout the file (ata started in sosNF with 4-column
indents and other style bugs, but now has a mangled mess of sosNF and
KNF indents and other style bugs).
> Modified: head/sys/netpfil/ipfw/ip_fw_sockopt.c
> ==============================================================================
> --- head/sys/netpfil/ipfw/ip_fw_sockopt.c Sun Apr 13 20:21:56 2014 (r264420)
> +++ head/sys/netpfil/ipfw/ip_fw_sockopt.c Sun Apr 13 21:13:33 2014 (r264421)
> @@ -1039,8 +1039,10 @@ ipfw_ctl(struct sockopt *sopt)
> if (sopt->sopt_valsize == RULESIZE7(rule)) {
> is7 = 1;
> error = convert_rule_to_8(rule);
> - if (error)
> + if (error) {
> + free(rule, M_TEMP);
> return error;
> + }
> if (error == 0)
> error = check_ipfw_struct(rule, RULESIZE(rule));
> } else {
Typical 4-column indents in is7 code.
> @@ -1056,11 +1058,13 @@ ipfw_ctl(struct sockopt *sopt)
> if (is7) {
> error = convert_rule_to_7(rule);
> size = RULESIZE7(rule);
> - if (error)
> + if (error) {
> + free(rule, M_TEMP);
> return error;
> + }
Atypical (for is7) normal 8-column indents in is7 code. This is the only
is7 block in the file that has normal indents. is7 code in other ipfw
files has notmal indents.
> }
> error = sooptcopyout(sopt, rule, size);
> - }
> + }
> }
> free(rule, M_TEMP);
> break;
ipfw code has mounds of other style bugs, so running it through indent
makes too many changes to easily find the mere indentation bugs.
According to knfom:
%%%
79.069% dn_heap.h
77.876% dn_sched.h
62.386% ip_dn_private.h
69.688% ip_fw_private.h
79.533% dn_heap.c
75.021% dn_sched_fifo.c
81.667% dn_sched_prio.c
Error@230: Unbalanced parens
Warning@232: Extra )
Error@541: Unbalanced parens
Warning@541: Extra )
Error@544: Unbalanced parens
Warning@544: Extra )
Error@558: Unbalanced parens
Warning@558: Extra )
Error@589: Unbalanced parens
Warning@589: Extra )
Error@642: Unbalanced parens
Warning@642: Extra )
Error@649: Unbalanced parens
Warning@649: Extra )
80.731% dn_sched_qfq.c
82.742% dn_sched_rr.c
55.642% dn_sched_wf2q.c
71.398% ip_dn_glue.c
84.747% ip_dn_io.c
Warning@1283: Extra )
Warning@1286: Extra )
Error@1537: Statement nesting error
Error@2307: Stuff missing from end of file
57.207% ip_dummynet.c
67.913% ip_fw2.c
Error@621: Unbalanced parens
Warning@621: Extra )
Error@658: Unbalanced parens
Warning@658: Extra )
Error@679: Unbalanced parens
Warning@679: Extra )
Error@689: Unbalanced parens
Warning@693: Extra )
Error@722: Unbalanced parens
Warning@726: Extra )
Error@1139: Unbalanced parens
Warning@1140: Extra )
Error@1147: Unbalanced parens
Warning@1148: Extra )
Error@1243: Unbalanced parens
Warning@1244: Extra )
83.301% ip_fw_dynamic.c
84.102% ip_fw_log.c
96.352% ip_fw_nat.c
70.911% ip_fw_pfil.c
84.409% ip_fw_sockopt.c
93.064% ip_fw_table.c
%%%
90% would be mediocre (but well above average). 95% would be very good.
90% means that indent with no block comments reformatted and certain
other options creates (non-context) diffs of size 10% of the original
file. ip_fw_table.c has 764 lines, so 93% for it means about 54 lines
of diffs or about 14 lines changed if the changes are scattered.
The errors are because ipfw uses many C90 and later constructions that
are not understood by indent. It has 58 lines with C++ style comments...
It is still closer to KNF than ata, since consistent 4-column indents
give a style bug on almost every non-blank line:
66.105% ata-all.h
89.168% ata-pci.h
52.018% ata-all.c
45.705% ata-card.c
37.662% ata-cbus.c
29.365% ata-dma.c
42.298% ata-isa.c
15.969% ata-lowlevel.c
44.266% ata-pci.c
29.038% ata-sata.c
ata has only 2 C++ style comments. kern still has none. Complicated
declarations are more likely than comments to be the cause of the errors.
ipfw has mounds of non-whitespace errors which indent cannot always
fix. The most obvious ones in the above are 'return error;' (indent
can fix this) and 'if (error)' (indent can't fix this).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140414170654.Y5260>