From owner-svn-src-all@FreeBSD.ORG Mon Apr 14 07:35:46 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 26B015F3; Mon, 14 Apr 2014 07:35:46 +0000 (UTC) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id C56351CD3; Mon, 14 Apr 2014 07:35:45 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id A19861043D6D; Mon, 14 Apr 2014 17:35:35 +1000 (EST) Date: Mon, 14 Apr 2014 17:35:32 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Christian Brueffer Subject: Re: svn commit: r264421 - head/sys/netpfil/ipfw In-Reply-To: <201404132113.s3DLDXRI044236@svn.freebsd.org> Message-ID: <20140414170654.Y5260@besplex.bde.org> References: <201404132113.s3DLDXRI044236@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=U6SrU4bu c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=Fwfa6FMV41EA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=1f4-qKUDpKhKomwRpnoA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Apr 2014 07:35:46 -0000 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