Re: [PATCH] i386-pc: build verifiers API as module

[Colin Watson <cjwatson@debian.org>]

On Thu, Mar 18, 2021 at 03:14:34PM +0800, Michael Chang via Grub-devel wrote:
> Given no core functions on i386-pc would require verifiers to work and
> the only consumer of the verifier API is the pgp module, it looks good
> to me that we can move the verifiers out of the kernel image and let
> moddep.lst to auto-load it when pgp is loaded on i386-pc platform.
> 
> This helps to reduce the size of core image and thus can relax the
> tension of exploading on some i386-pc system with very short MBR gap
> size. See also a very comprehensive summary from Colin [1] about the
> details.

Thanks for working on this!  It's certainly awkward to have to deal with
this sort of thing, but apparently not as awkward as I'd feared, and
it's better than the alternative.

> +AM_CONDITIONAL([COND_NOT_i386_pc], [test x$target_cpu != xi386 -o x$platform != xpc])

You could drop this and instead just do "if !COND_i386_pc" in
grub-core/Makefile.am.

> diff --git a/grub-core/kern/verifiers.c b/grub-core/kern/verifiers.c
> index 75d7994cf..85887917d 100644
> --- a/grub-core/kern/verifiers.c
> +++ b/grub-core/kern/verifiers.c
> @@ -221,8 +221,19 @@ grub_verify_string (char *str, enum grub_verify_string_type type)
>    return GRUB_ERR_NONE;
>  }
>  
> +#ifdef GRUB_MACHINE_PCBIOS
> +GRUB_MOD_INIT(verifiers)
> +#else
>  void
>  grub_verifiers_init (void)
> +#endif

I think a comment here (or somewhere in the actual code, anyway, I don't
mind where) would be useful so that people trying to work out what's
going on don't have to hunt through commit logs to find out.

If these minor comments are fixed:

Reviewed-by: Colin Watson <cjwatson@debian.org>

-- 
Colin Watson (he/him)                              [cjwatson@debian.org]