c99ism in latest linus -bk

c99ism in latest linus -bk

[Andrew Morton]

net/sched/cls_api.c: In function `tcf_exts_validate':
net/sched/cls_api.c:489: parse error before `int'
net/sched/cls_api.c:493: `act' undeclared (first use in this function)
net/sched/cls_api.c:493: (Each undeclared identifier is reported only once
net/sched/cls_api.c:493: for each function it appears in.)
net/sched/cls_api.c:494: `err' undeclared (first use in this function)

Someone wanna fix that up please?

[PATCH] PKT_SCHED: remove c99ism

[Thomas Graf]

* Andrew Morton <20050115144010.33182075.akpm@osdl.org> 2005-01-15 14:40
> net/sched/cls_api.c: In function `tcf_exts_validate':
> net/sched/cls_api.c:489: parse error before `int'
> net/sched/cls_api.c:493: `act' undeclared (first use in this function)
> net/sched/cls_api.c:493: (Each undeclared identifier is reported only once
> net/sched/cls_api.c:493: for each function it appears in.)
> net/sched/cls_api.c:494: `err' undeclared (first use in this function)

Signed-off-by: Thomas Graf <tgraf@suug.ch>

--- linux-2.6.11-rc1-bk2.orig/net/sched/cls_api.c	2005-01-16 00:04:40.000000000 +0100
+++ linux-2.6.11-rc1-bk2/net/sched/cls_api.c	2005-01-15 23:56:48.000000000 +0100
@@ -486,10 +486,9 @@
 	memset(exts, 0, sizeof(*exts));
 	
 #ifdef CONFIG_NET_CLS_ACT
-	int err;
-	struct tc_action *act;
-
 	if (map->police && tb[map->police-1]) {
+		int err;
+		struct tc_action *act;
 		act = tcf_action_init_1(tb[map->police-1], rate_tlv, "police",
 			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
 		if (act == NULL)
@@ -498,6 +497,8 @@
 		act->type = TCA_OLD_COMPAT;
 		exts->action = act;
 	} else if (map->action && tb[map->action-1]) {
+		int err;
+		struct tc_action *act;
 		act = tcf_action_init(tb[map->action-1], rate_tlv, NULL,
 			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
 		if (act == NULL)

Re: [PATCH] PKT_SCHED: remove c99ism

[Andrew Morton]

Thomas Graf <tgraf@suug.ch> wrote:
>
>  #ifdef CONFIG_NET_CLS_ACT
>  -	int err;
>  -	struct tc_action *act;
>  -
>   	if (map->police && tb[map->police-1]) {
>  +		int err;
>  +		struct tc_action *act;
>   		act = tcf_action_init_1(tb[map->police-1], rate_tlv, "police",
>   			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
>   		if (act == NULL)
>  @@ -498,6 +497,8 @@
>   		act->type = TCA_OLD_COMPAT;
>   		exts->action = act;
>   	} else if (map->action && tb[map->action-1]) {
>  +		int err;
>  +		struct tc_action *act;
>   		act = tcf_action_init(tb[map->action-1], rate_tlv, NULL,
>   			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
>   		if (act == NULL)

yes, that's the obvious one, but it uses a little more stack space.

An uglier but more efficient approach is to whack braces around the whole
thing.

[PATCH] PKT_SCHED: remove c99ism

[Thomas Graf]

* Andrew Morton <20050115153202.2f7f81c6.akpm@osdl.org> 2005-01-15 15:32
> An uglier but more efficient approach is to whack braces around the whole
> thing.

Fine with me as well. The code isn't used that often and not deep in the
calling stack which is why I preferred the cleaner way first.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

--- linux-2.6.11-rc1-bk2.orig/net/sched/cls_api.c	2005-01-16 00:04:40.000000000 +0100
+++ linux-2.6.11-rc1-bk2/net/sched/cls_api.c	2005-01-16 00:34:53.000000000 +0100
@@ -486,24 +486,26 @@
 	memset(exts, 0, sizeof(*exts));
 	
 #ifdef CONFIG_NET_CLS_ACT
-	int err;
-	struct tc_action *act;
+	{
+		int err;
+		struct tc_action *act;
 
-	if (map->police && tb[map->police-1]) {
-		act = tcf_action_init_1(tb[map->police-1], rate_tlv, "police",
-			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
-		if (act == NULL)
-			return err;
-
-		act->type = TCA_OLD_COMPAT;
-		exts->action = act;
-	} else if (map->action && tb[map->action-1]) {
-		act = tcf_action_init(tb[map->action-1], rate_tlv, NULL,
-			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
-		if (act == NULL)
-			return err;
+		if (map->police && tb[map->police-1]) {
+			act = tcf_action_init_1(tb[map->police-1], rate_tlv, "police",
+				TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
+			if (act == NULL)
+				return err;
+
+			act->type = TCA_OLD_COMPAT;
+			exts->action = act;
+		} else if (map->action && tb[map->action-1]) {
+			act = tcf_action_init(tb[map->action-1], rate_tlv, NULL,
+				TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
+			if (act == NULL)
+				return err;
 
-		exts->action = act;
+			exts->action = act;
+		}
 	}
 #elif defined CONFIG_NET_CLS_POLICE
 	if (map->police && tb[map->police-1]) {

Re: [PATCH] PKT_SCHED: remove c99ism

[Herbert Poetzl]

On Sun, Jan 16, 2005 at 12:40:56AM +0100, Thomas Graf wrote:
> * Andrew Morton <20050115153202.2f7f81c6.akpm@osdl.org> 2005-01-15 15:32
> > An uglier but more efficient approach is to whack braces around the whole
> > thing.
> 
> Fine with me as well. The code isn't used that often and not deep in the
> calling stack which is why I preferred the cleaner way first.

hmm, please remind me why putting some

#ifdef CONFIG_NET_CLS_ACT
	int err;
	struct tc_action *act;
#endif

at the _beginning_ of that procedure would
be a bad choice?

TIA,
Herbert

> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> 
> --- linux-2.6.11-rc1-bk2.orig/net/sched/cls_api.c	2005-01-16 00:04:40.000000000 +0100
> +++ linux-2.6.11-rc1-bk2/net/sched/cls_api.c	2005-01-16 00:34:53.000000000 +0100
> @@ -486,24 +486,26 @@
>  	memset(exts, 0, sizeof(*exts));
>  	
>  #ifdef CONFIG_NET_CLS_ACT
> -	int err;
> -	struct tc_action *act;
> +	{
> +		int err;
> +		struct tc_action *act;
>  
> -	if (map->police && tb[map->police-1]) {
> -		act = tcf_action_init_1(tb[map->police-1], rate_tlv, "police",
> -			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
> -		if (act == NULL)
> -			return err;
> -
> -		act->type = TCA_OLD_COMPAT;
> -		exts->action = act;
> -	} else if (map->action && tb[map->action-1]) {
> -		act = tcf_action_init(tb[map->action-1], rate_tlv, NULL,
> -			TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
> -		if (act == NULL)
> -			return err;
> +		if (map->police && tb[map->police-1]) {
> +			act = tcf_action_init_1(tb[map->police-1], rate_tlv, "police",
> +				TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
> +			if (act == NULL)
> +				return err;
> +
> +			act->type = TCA_OLD_COMPAT;
> +			exts->action = act;
> +		} else if (map->action && tb[map->action-1]) {
> +			act = tcf_action_init(tb[map->action-1], rate_tlv, NULL,
> +				TCA_ACT_NOREPLACE, TCA_ACT_BIND, &err);
> +			if (act == NULL)
> +				return err;
>  
> -		exts->action = act;
> +			exts->action = act;
> +		}
>  	}
>  #elif defined CONFIG_NET_CLS_POLICE
>  	if (map->police && tb[map->police-1]) {

Re: [PATCH] PKT_SCHED: remove c99ism

[Thomas Graf]

* Herbert Poetzl <20050117090912.GB30371@mail.13thfloor.at> 2005-01-17 10:09
> On Sun, Jan 16, 2005 at 12:40:56AM +0100, Thomas Graf wrote:
> > * Andrew Morton <20050115153202.2f7f81c6.akpm@osdl.org> 2005-01-15 15:32
> > > An uglier but more efficient approach is to whack braces around the whole
> > > thing.
> > 
> > Fine with me as well. The code isn't used that often and not deep in the
> > calling stack which is why I preferred the cleaner way first.
> 
> hmm, please remind me why putting some
> 
> #ifdef CONFIG_NET_CLS_ACT
> 	int err;
> 	struct tc_action *act;
> #endif
> 
> at the _beginning_ of that procedure would
> be a bad choice?

I personally don't think it's a bad choice but the less ifdefs the
better. 2 out of 3 of my latest bugs were related to such ifdefs because
testing gets more error prone. Personally I think the cleanest way
would be to create inlined functions together with a nop macro but
it would hide the mutual exclusiveness of NET_CLS_ACT and NET_CLS_POLICE
in this case which is even worse.

Re: [PATCH] PKT_SCHED: remove c99ism

[David S. Miller]

On Sun, 16 Jan 2005 00:40:56 +0100
Thomas Graf <tgraf@suug.ch> wrote:

> * Andrew Morton <20050115153202.2f7f81c6.akpm@osdl.org> 2005-01-15 15:32
> > An uglier but more efficient approach is to whack braces around the whole
> > thing.
> 
> Fine with me as well. The code isn't used that often and not deep in the
> calling stack which is why I preferred the cleaner way first.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

Applied, thanks Thomas.