svn commit: r183881 - head/sys/netinet

[Bruce Evans]

On Tue, 14 Oct 2008, Max Laier wrote:

> On Tuesday 14 October 2008 16:16:05 Roman Kurakin wrote:
>>> ...
>>> Log:
>>>   o Reformat ipfw nat get|setsockopt code to look it  more
>>>   style(9) compliant.  No functional changes.
>>>
>>> Modified:
>>>   head/sys/netinet/ip_fw2.c
>>>
>>> Modified: head/sys/netinet/ip_fw2.c
>>> =========================================================================
>>> ===== --- head/sys/netinet/ip_fw2.c	Tue Oct 14 10:23:11 2008	(r183880) +++
>>> head/sys/netinet/ip_fw2.c	Tue Oct 14 12:26:55 2008	(r183881) @@ -4385,49
>>> +4385,52 @@ ipfw_ctl(struct sockopt *sopt)
>>>  		break;
>>>
>>>  	case IP_FW_NAT_CFG:
>>> -	{
>>> -		if (IPFW_NAT_LOADED)
>>> -			error = ipfw_nat_cfg_ptr(sopt);
>>> -		else {
>>> -			printf("IP_FW_NAT_CFG: ipfw_nat not present, please load it.\n");
>>> -			error = EINVAL;
>>> +		{
>>> +			if (IPFW_NAT_LOADED)
>>> +				error = ipfw_nat_cfg_ptr(sopt);
>>> ...
>>
>> IMHO such indention does not add any usefulness, but increases indention
>> level that is already very high.
>> Also I do not see strict contradiction to the style(9), but probably I
>> am not reading the most current style(9).

style(9) certainly requires indentation for each level of {}.

> The additional scope is absolutely unnecessary here and should be dropped -
> IMHO.  See case IP_FW_RESETLOG and above.

The bogus {} are probably left over from another style bug -- nested
declarations.  One reason that style(9) forbids nested declarations is
that they require the extra scope (except now using C99 (*)).  Too-large
case statements sometimes want to use different local variables for
each case.  To prevent the indentation reaching column 800, the formatting
in the original version of the above is sometimes used.  But this is just
abnother layer of style bugs if the case statement doesn't even have local
variables.

(*) In C99, you can write:

 	int foo;
 	use (foo);

too easily and not even notice that to properly violate style(9)'s nesting
rule you should have written:

 	{				/* bletch */
 		int foo;

 		use (foo);
 	}

or that to not violate style(9) you should have written:

 	int a, b, c, d, e, f, foo, ...;	/* sort foo into function's decls */
 	...
 	use(foo);

style(9) doesn't allow the C99 declaration since C99 hasn't noticed that
C99 exists.

Bruce