Re: [Xen-devel] [PATCH V3] Decouple SnadyBridge quirk form VTd timeout

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.11.14 at 20:46, <donald.d.dugger@xxxxxxxxx> wrote:
> @@ -237,6 +248,42 @@
>      }
>  }
>  
> +static void __init parse_snb_timeout(const char *s)
> +{
> +     int not;
> +
> +     switch (*s) {
> +
> +     case '\0':
> +             snb_igd_timeout = SNB_IGD_TIMEOUT_LEGACY;
> +             break;
> +
> +     case '0':       case '1':       case '2':
> +     case '3':       case '4':       case '5':
> +     case '6':       case '7':       case '8':
> +     case '9':
> +             snb_igd_timeout = MILLISECS(simple_strtoul(s, &s, 0));
> +             if ( snb_igd_timeout == MILLISECS(1) )
> +                     snb_igd_timeout = SNB_IGD_TIMEOUT_LEGACY;
> +             break;

Overly complicated. Just parse_bool() first, if that returns negative
check for "default" or "", and (if not matched) invoke strtoul(). No
need for this switch statement.

> +
> +     default:
> +             if ( strncmp("default", s, 7) == 0 ) {
> +                     snb_igd_timeout = SNB_IGD_TIMEOUT;
> +                     break;
> +             }
> +             not = !strncmp("no-", s, 3);

This makes no sense - you're looking for e.g. "snb_igd_quirk=no-no"
here. If the use specified "no-snb_igd_quirk", you'll end up seeing
"=no" when this function gets entered.

Also the whole function is white space damaged (using hard tabs)
and has misplaced opening braces.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel