[PATCH v2 1/1] efi_loader: prepare for read only OP-TEE variables

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Wed, Jun 24, 2020 at 07:51:42AM +0200, Heinrich Schuchardt wrote:
> On 6/23/20 1:44 AM, AKASHI Takahiro wrote:
> > On Mon, Jun 22, 2020 at 06:10:27PM +0200, Heinrich Schuchardt wrote:
> >> We currently have two implementations of UEFI variables:
> >>
> >> * variables provided via an OP-TEE module
> >> * variables stored in the U-Boot environment
> >>
> >> Read only variables are up to now only implemented in the U-Boot
> >> environment implementation.
> >>
> >> Provide a common interface for both implementations that allows handling
> >> read-only variables.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >> v2:
> >> 	add missing efi_variable.h
> >> 	consider attributes==NULL in efi_variable_get()
> >> ---
> >>  include/efi_variable.h               |  40 +++++++
> >>  lib/efi_loader/Makefile              |   1 +
> >>  lib/efi_loader/efi_variable.c        | 171 ++++++++-------------------
> >>  lib/efi_loader/efi_variable_common.c |  79 +++++++++++++
> >>  lib/efi_loader/efi_variable_tee.c    |  75 ++++--------
> >>  5 files changed, 188 insertions(+), 178 deletions(-)
> >>  create mode 100644 include/efi_variable.h
> >>  create mode 100644 lib/efi_loader/efi_variable_common.c
> >>
> >> diff --git a/include/efi_variable.h b/include/efi_variable.h
> >> new file mode 100644
> >> index 0000000000..784dbd9cf4
> >> --- /dev/null
> >> +++ b/include/efi_variable.h
> >
> > I think that all the stuff here should be put in efi_loader.h.
> > I don't see any benefit of having a separate header.
> >
> >
> 
> This is more or less a question of taste. My motivation is:

I can agree, but at the same time, I don't like such an ad-hoc
confusing approach. I think that you should have a firm discipline.

> * efi_loader.h is rather large (805 lines).
> * Other variable functions will be added.
> * The functions defined here are used only in very few places
>   while efi_loader.h is included in 57 files.

If we allow this, we will have a number of small headers,
which will contradict a notion of efi_loader.h.

-Takahiro Akashi

> Best regards
> 
> Heinrich