[uml-devel] [RFC PATCH] uml: fix double statement if missing braces

[uml-devel] [RFC PATCH] uml: fix double statement if missing braces

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 821 bytes --]

...I'm not fully sure what's the intention here, ie., whether
the return belongs to a block with the assignment or not.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 arch/um/os-Linux/sys-i386/tls.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/um/os-Linux/sys-i386/tls.c b/arch/um/os-Linux/sys-i386/tls.c
index 32ed41e..2a1412d 100644
--- a/arch/um/os-Linux/sys-i386/tls.c
+++ b/arch/um/os-Linux/sys-i386/tls.c
@@ -24,11 +24,12 @@ void check_host_supports_tls(int *supports_tls, int *tls_min) {
 			*supports_tls = 1;
 			return;
 		} else {
-			if (errno == EINVAL)
+			if (errno == EINVAL) {
 				continue;
-			else if (errno == ENOSYS)
+			} else if (errno == ENOSYS) {
 				*supports_tls = 0;
 				return;
+			}
 		}
 	}
 
-- 
1.5.2.2

[-- Attachment #2: Type: text/plain, Size: 363 bytes --]

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[-- Attachment #3: Type: text/plain, Size: 194 bytes --]

_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [RFC PATCH] uml: fix double statement if missing braces

[Jeff Dike]

On Tue, Aug 19, 2008 at 09:09:16AM +0300, Ilpo Järvinen wrote:
> ...I'm not fully sure what's the intention here, ie., whether
> the return belongs to a block with the assignment or not.

Yes, this is confused, although it just happens to compile to
something sane.  Your patch is fine, but I think the whole thing needs
some more cleanup.  See the patch below.

     	  	    	Jeff

-- 
Work email - jdike at linux dot intel dot com

The testing for the host's TLS support was somewhat confused - notably
in the indentation of a return.

This cleans it up:
	*supports_tls is only set to 0 once
	any error besides EINVAL causes an immediate return, with
*supports_tls equal to 0.

Index: linux-2.6.22/arch/um/os-Linux/sys-i386/tls.c
===================================================================
--- linux-2.6.22.orig/arch/um/os-Linux/sys-i386/tls.c	2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.22/arch/um/os-Linux/sys-i386/tls.c	2008-08-26 08:19:37.000000000 -0400
@@ -15,6 +15,7 @@ void check_host_supports_tls(int *suppor
 	int val[] = {GDT_ENTRY_TLS_MIN_I386, GDT_ENTRY_TLS_MIN_X86_64};
 	int i;
 
+	*supports_tls = 0;
 	for (i = 0; i < ARRAY_SIZE(val); i++) {
 		user_desc_t info;
 		info.entry_number = val[i];
@@ -23,14 +24,9 @@ void check_host_supports_tls(int *suppor
 			*tls_min = val[i];
 			*supports_tls = 1;
 			return;
-		} else {
-			if (errno == EINVAL)
-				continue;
-			else if (errno == ENOSYS)
-				*supports_tls = 0;
-				return;
 		}
+		if (errno == EINVAL)
+			continue;
+		else return;
 	}
-
-	*supports_tls = 0;
 }

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [RFC PATCH] uml: fix double statement if missing braces

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2199 bytes --]

On Tue, 26 Aug 2008, Jeff Dike wrote:

> On Tue, Aug 19, 2008 at 09:09:16AM +0300, Ilpo Järvinen wrote:
> > ...I'm not fully sure what's the intention here, ie., whether
> > the return belongs to a block with the assignment or not.
> 
> Yes, this is confused, although it just happens to compile to
> something sane.

Yeah, I found ~20 similar brace vs reindent cases, for most of them (like 
this), it's currently insignificant but definately is trap for 1) a reader 
of the code and 2) future modification.

> Your patch is fine, but I think the whole thing needs
> some more cleanup.  See the patch below.

Seems much better than my approach, this case was anyway among the most 
complex within those ~20 case what comes to figuring out correctness, 
after your cleanup the code flow much more obvious.

-- 
> The testing for the host's TLS support was somewhat confused - notably
> in the indentation of a return.
> 
> This cleans it up:
> 	*supports_tls is only set to 0 once
> 	any error besides EINVAL causes an immediate return, with
> *supports_tls equal to 0.
> 
> Index: linux-2.6.22/arch/um/os-Linux/sys-i386/tls.c
> ===================================================================
> --- linux-2.6.22.orig/arch/um/os-Linux/sys-i386/tls.c	2007-07-08 19:32:17.000000000 -0400
> +++ linux-2.6.22/arch/um/os-Linux/sys-i386/tls.c	2008-08-26 08:19:37.000000000 -0400
> @@ -15,6 +15,7 @@ void check_host_supports_tls(int *suppor
>  	int val[] = {GDT_ENTRY_TLS_MIN_I386, GDT_ENTRY_TLS_MIN_X86_64};
>  	int i;
>  
> +	*supports_tls = 0;
>  	for (i = 0; i < ARRAY_SIZE(val); i++) {
>  		user_desc_t info;
>  		info.entry_number = val[i];
> @@ -23,14 +24,9 @@ void check_host_supports_tls(int *suppor
>  			*tls_min = val[i];
>  			*supports_tls = 1;
>  			return;
> -		} else {
> -			if (errno == EINVAL)
> -				continue;
> -			else if (errno == ENOSYS)
> -				*supports_tls = 0;
> -				return;
>  		}
> +		if (errno == EINVAL)
> +			continue;
> +		else return;

They usually say that you might want to put the return on a different line 
unless you have something to hide :-).

>  	}
> -
> -	*supports_tls = 0;
>  }
> 

-- 
 i.

[-- Attachment #2: Type: text/plain, Size: 363 bytes --]

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[-- Attachment #3: Type: text/plain, Size: 194 bytes --]

_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] [RFC PATCH] uml: fix double statement if missing braces

[Jeff Dike]

On Wed, Aug 27, 2008 at 12:34:08PM +0300, Ilpo Järvinen wrote:
> They usually say that you might want to put the return on a different line 
> unless you have something to hide :-).

Whoops, right you are.

	      	  Jeff

-- 
Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel