diff mbox series

[v3,01/13] lib/crypto, efi_loader: avoid multiple inclusions of header files

Message ID 20200708050203.15230-2-takahiro.akashi@linaro.org
State New
Headers show
Series efi_loader: rework/improve UEFI secure boot code | expand

Commit Message

AKASHI Takahiro July 8, 2020, 5:01 a.m. UTC
By adding extra symbols, we can now avoid including x509_parser and
pkcs7_parser.h files multiple times.

Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
 lib/efi_loader/efi_image_loader.c | 1 +
 lib/efi_loader/efi_signature.c    | 1 +
 2 files changed, 2 insertions(+)

Comments

Heinrich Schuchardt July 8, 2020, 10:29 a.m. UTC | #1
On 08.07.20 07:01, AKASHI Takahiro wrote:
> By adding extra symbols, we can now avoid including x509_parser and
> pkcs7_parser.h files multiple times.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  lib/efi_loader/efi_image_loader.c | 1 +
>  lib/efi_loader/efi_signature.c    | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index 06a2ebdb9081..5b00fea2f113 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -14,6 +14,7 @@
>  #include <pe.h>
>  #include <sort.h>
>  #include <crypto/pkcs7_parser.h>
> +#include <crypto/x509_parser.h>
>  #include <linux/err.h>
>
>  const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> index e05c471c61ce..da3818ac62e2 100644
> --- a/lib/efi_loader/efi_signature.c
> +++ b/lib/efi_loader/efi_signature.c
> @@ -11,6 +11,7 @@
>  #include <hexdump.h>
>  #include <malloc.h>
>  #include <crypto/pkcs7_parser.h>
> +#include <crypto/x509_parser.h>
>  #include <linux/compat.h>
>  #include <linux/oid_registry.h>
>  #include <u-boot/rsa.h>
>

I would prefer to use

[PATCH 1/1] lib/crypto: use qualified path for x509_parser.h
https://lists.denx.de/pipermail/u-boot/2020-July/419214.html

which has the same effect.

Best regards

Heinrich
AKASHI Takahiro July 9, 2020, 12:47 a.m. UTC | #2
Heinrich,

On Wed, Jul 08, 2020 at 12:29:54PM +0200, Heinrich Schuchardt wrote:
> On 08.07.20 07:01, AKASHI Takahiro wrote:
> > By adding extra symbols, we can now avoid including x509_parser and
> > pkcs7_parser.h files multiple times.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > ---
> >  lib/efi_loader/efi_image_loader.c | 1 +
> >  lib/efi_loader/efi_signature.c    | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> > index 06a2ebdb9081..5b00fea2f113 100644
> > --- a/lib/efi_loader/efi_image_loader.c
> > +++ b/lib/efi_loader/efi_image_loader.c
> > @@ -14,6 +14,7 @@
> >  #include <pe.h>
> >  #include <sort.h>
> >  #include <crypto/pkcs7_parser.h>
> > +#include <crypto/x509_parser.h>
> >  #include <linux/err.h>
> >
> >  const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
> > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> > index e05c471c61ce..da3818ac62e2 100644
> > --- a/lib/efi_loader/efi_signature.c
> > +++ b/lib/efi_loader/efi_signature.c
> > @@ -11,6 +11,7 @@
> >  #include <hexdump.h>
> >  #include <malloc.h>
> >  #include <crypto/pkcs7_parser.h>
> > +#include <crypto/x509_parser.h>
> >  #include <linux/compat.h>
> >  #include <linux/oid_registry.h>
> >  #include <u-boot/rsa.h>
> >

The patch (and the commit message) seems to have been
somehow corrupted during my rework. 

> I would prefer to use
> 
> [PATCH 1/1] lib/crypto: use qualified path for x509_parser.h
> https://lists.denx.de/pipermail/u-boot/2020-July/419214.html

My point is, as I said before, that we should include a header
*directly* if some symbols in that header are used in a given file
even though such a header is *indirectly* inherited by another
header, pkcs7_parser.h in this case.

That way, "#include" explicitly indicates that the C file relies on
another header and associated functions.
I always take this practice.

In this sense, this patch still makes sense.

-Takahiro Akashi

> 
> which has the same effect.
> 
> Best regards
> 
> Heinrich
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 06a2ebdb9081..5b00fea2f113 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -14,6 +14,7 @@ 
 #include <pe.h>
 #include <sort.h>
 #include <crypto/pkcs7_parser.h>
+#include <crypto/x509_parser.h>
 #include <linux/err.h>
 
 const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
index e05c471c61ce..da3818ac62e2 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -11,6 +11,7 @@ 
 #include <hexdump.h>
 #include <malloc.h>
 #include <crypto/pkcs7_parser.h>
+#include <crypto/x509_parser.h>
 #include <linux/compat.h>
 #include <linux/oid_registry.h>
 #include <u-boot/rsa.h>