Re: [PATCH] x86/efi: Auto enable EFI memmap on SGI UV systems

[Ingo Molnar]

* Joseph Thelen <jthe...@sgi.com> wrote:

>  static int __init setup_add_efi_memmap(char *arg)
>  {
> +     static bool arg_as_bool;

Why hidden inside local variables as static?

> +     int ret = strtobool(arg, &arg_as_bool);
> +
> +     /* check for a non-existent arg, to maintain backward compatibility */
> +     if (!arg) {
> +             add_efi_memmap = EFI_MEMMAP_ENABLED;
> +     } else {
> +             if (ret) {
> +                     /* a bad argument was passed... */
> +                     return ret;
> +             } else {
> +                     if (arg_as_bool)
> +                             add_efi_memmap = EFI_MEMMAP_ENABLED;
> +                     else
> +                             add_efi_memmap = EFI_MEMMAP_DISABLED;
> +             }
> +     }
> +
>       return 0;

And that's a really weird code flow!

How about something straightforward:

        int val = 0;
        int ret;

        /* Check for a non-existent arg, to maintain backward compatibility: */
        if (!arg) {
                add_efi_memmap = EFI_MEMMAP_ENABLED;
                return 0;
        }

        ret = strtobool(arg, &val);

        /* Was a bad argument passed? */
        if (ret)
                return ret;

        if (val)
                add_efi_memmap = EFI_MEMMAP_ENABLED;
        else
                add_efi_memmap = EFI_MEMMAP_DISABLED;

        return 0;

?

Also note the rename to 'val'.

Thanks,

        Ingo