svn commit: r264421 - head/sys/netpfil/ipfw

[Bruce Evans]

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 at 230: Unbalanced parens
Warning at 232: Extra )
Error at 541: Unbalanced parens
Warning at 541: Extra )
Error at 544: Unbalanced parens
Warning at 544: Extra )
Error at 558: Unbalanced parens
Warning at 558: Extra )
Error at 589: Unbalanced parens
Warning at 589: Extra )
Error at 642: Unbalanced parens
Warning at 642: Extra )
Error at 649: Unbalanced parens
Warning at 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 at 1283: Extra )
Warning at 1286: Extra )
Error at 1537: Statement nesting error
Error at 2307: Stuff missing from end of file
  57.207% ip_dummynet.c
  67.913% ip_fw2.c
Error at 621: Unbalanced parens
Warning at 621: Extra )
Error at 658: Unbalanced parens
Warning at 658: Extra )
Error at 679: Unbalanced parens
Warning at 679: Extra )
Error at 689: Unbalanced parens
Warning at 693: Extra )
Error at 722: Unbalanced parens
Warning at 726: Extra )
Error at 1139: Unbalanced parens
Warning at 1140: Extra )
Error at 1147: Unbalanced parens
Warning at 1148: Extra )
Error at 1243: Unbalanced parens
Warning at 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