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.
-------------------------------------------------------------------------
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