* [uml-devel] [RFC PATCH] uml: fix double statement if missing braces @ 2008-08-19 6:09 Ilpo Järvinen 2008-08-26 16:52 ` Jeff Dike 0 siblings, 1 reply; 4+ messages in thread From: Ilpo Järvinen @ 2008-08-19 6:09 UTC (permalink / raw) To: jdike; +Cc: user-mode-linux-devel [-- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [uml-devel] [RFC PATCH] uml: fix double statement if missing braces 2008-08-19 6:09 [uml-devel] [RFC PATCH] uml: fix double statement if missing braces Ilpo Järvinen @ 2008-08-26 16:52 ` Jeff Dike 2008-08-27 9:34 ` Ilpo Järvinen 0 siblings, 1 reply; 4+ messages in thread From: Jeff Dike @ 2008-08-26 16:52 UTC (permalink / raw) To: Ilpo Järvinen; +Cc: user-mode-linux-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [uml-devel] [RFC PATCH] uml: fix double statement if missing braces 2008-08-26 16:52 ` Jeff Dike @ 2008-08-27 9:34 ` Ilpo Järvinen 2008-08-27 15:09 ` Jeff Dike 0 siblings, 1 reply; 4+ messages in thread From: Ilpo Järvinen @ 2008-08-27 9:34 UTC (permalink / raw) To: Jeff Dike; +Cc: user-mode-linux-devel [-- 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [uml-devel] [RFC PATCH] uml: fix double statement if missing braces 2008-08-27 9:34 ` Ilpo Järvinen @ 2008-08-27 15:09 ` Jeff Dike 0 siblings, 0 replies; 4+ messages in thread From: Jeff Dike @ 2008-08-27 15:09 UTC (permalink / raw) To: Ilpo Järvinen; +Cc: user-mode-linux-devel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-08-27 15:09 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-08-19 6:09 [uml-devel] [RFC PATCH] uml: fix double statement if missing braces Ilpo Järvinen 2008-08-26 16:52 ` Jeff Dike 2008-08-27 9:34 ` Ilpo Järvinen 2008-08-27 15:09 ` Jeff Dike
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.