| Message ID | 20200421003817.16078-1-takahiro.akashi@linaro.org |
|---|---|
| State | Accepted |
| Commit | e3f5c9cb0fcc95aa9287b5f8609294fe1a59b9da |
| Headers | show |
| Series | lib/crypto, efi_loader: move some headers to include/crypto | expand |
On 4/21/20 2:38 AM, AKASHI Takahiro wrote: > Pkcs7_parse.h and x509_parser.h are used in UEFI subsystem, in particular, > secure boot. So move them to include/crypto to avoid relative paths. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > --- > {lib => include}/crypto/pkcs7_parser.h | 0 > {lib => include}/crypto/x509_parser.h | 0 > lib/crypto/pkcs7_parser.c | 4 ++++ > lib/crypto/x509_cert_parser.c | 4 ++++ > lib/crypto/x509_public_key.c | 6 ++++-- > lib/efi_loader/efi_image_loader.c | 4 ++-- > lib/efi_loader/efi_signature.c | 4 ++-- > lib/efi_loader/efi_variable.c | 2 +- > test/lib/asn1.c | 4 ++-- > 9 files changed, 19 insertions(+), 9 deletions(-) > rename {lib => include}/crypto/pkcs7_parser.h (100%) > rename {lib => include}/crypto/x509_parser.h (100%) > > diff --git a/lib/crypto/pkcs7_parser.h b/include/crypto/pkcs7_parser.h > similarity index 100% > rename from lib/crypto/pkcs7_parser.h > rename to include/crypto/pkcs7_parser.h > diff --git a/lib/crypto/x509_parser.h b/include/crypto/x509_parser.h > similarity index 100% > rename from lib/crypto/x509_parser.h > rename to include/crypto/x509_parser.h > diff --git a/lib/crypto/pkcs7_parser.c b/lib/crypto/pkcs7_parser.c > index f5dda1179f8a..0ee207b6b1c8 100644 > --- a/lib/crypto/pkcs7_parser.c > +++ b/lib/crypto/pkcs7_parser.c > @@ -20,7 +20,11 @@ > #include <linux/err.h> > #include <linux/oid_registry.h> > #include <crypto/public_key.h> > +#ifdef __UBOOT__ > +#include <crypto/pkcs7_parser.h> > +#else > #include "pkcs7_parser.h" > +#endif > #include "pkcs7.asn1.h" > > MODULE_DESCRIPTION("PKCS#7 parser"); > diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c > index 4e41cffd2301..18f5407a076c 100644 > --- a/lib/crypto/x509_cert_parser.c > +++ b/lib/crypto/x509_cert_parser.c > @@ -18,7 +18,11 @@ > #include <linux/string.h> > #endif > #include <crypto/public_key.h> > +#ifdef __UBOOT__ > +#include <crypto/x509_parser.h> > +#else > #include "x509_parser.h" > +#endif > #include "x509.asn1.h" > #include "x509_akid.asn1.h" > > diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c > index 676c0df17410..571af9a0adf9 100644 > --- a/lib/crypto/x509_public_key.c > +++ b/lib/crypto/x509_public_key.c > @@ -16,15 +16,17 @@ > #include <linux/module.h> > #endif > #include <linux/kernel.h> > -#ifndef __UBOOT__ > +#ifdef __UBOOT__ > +#include <crypto/x509_parser.h> > +#else > #include <linux/slab.h> > #include <keys/asymmetric-subtype.h> > #include <keys/asymmetric-parser.h> > #include <keys/system_keyring.h> > #include <crypto/hash.h> > #include "asymmetric_keys.h" > -#endif > #include "x509_parser.h" > +#endif > > /* > * Set up the signature parameters in an X.509 certificate. This involves > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > index 899adf8493d0..f59b9a01140e 100644 > --- a/lib/efi_loader/efi_image_loader.c > +++ b/lib/efi_loader/efi_image_loader.c > @@ -13,8 +13,8 @@ > #include <malloc.h> > #include <pe.h> > #include <sort.h> > -#include "../lib/crypto/pkcs7_parser.h" > -#include "../lib/crypto/x509_parser.h" > +#include "crypto/pkcs7_parser.h" > +#include "crypto/x509_parser.h" Thanks for fixing this. x509_parser.h is included in pkcs7_parser.h. Please, remove the superfluous line. > > const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; > const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID; > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c > index 6ad09e4acbd7..0ead10203aeb 100644 > --- a/lib/efi_loader/efi_signature.c > +++ b/lib/efi_loader/efi_signature.c > @@ -10,12 +10,12 @@ > #include <image.h> > #include <hexdump.h> > #include <malloc.h> > +#include <crypto/pkcs7_parser.h> > +#include <crypto/x509_parser.h> Same here. Best regards Heinrich > #include <linux/compat.h> > #include <linux/oid_registry.h> > #include <u-boot/rsa.h> > #include <u-boot/sha256.h> > -#include "../lib/crypto/pkcs7_parser.h" > -#include "../lib/crypto/x509_parser.h" > > const efi_guid_t efi_guid_image_security_database = > EFI_IMAGE_SECURITY_DATABASE_GUID; > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index 7df881a74b44..0c6d1deb58eb 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -12,9 +12,9 @@ > #include <malloc.h> > #include <rtc.h> > #include <search.h> > +#include <crypto/pkcs7_parser.h> > #include <linux/compat.h> > #include <u-boot/crc.h> > -#include "../lib/crypto/pkcs7_parser.h" > > enum efi_secure_mode { > EFI_MODE_SETUP, > diff --git a/test/lib/asn1.c b/test/lib/asn1.c > index d2b3f67e68da..8661fdd30687 100644 > --- a/test/lib/asn1.c > +++ b/test/lib/asn1.c > @@ -13,10 +13,10 @@ > #include <test/ut.h> > > #ifdef CONFIG_PKCS7_MESSAGE_PARSER > -#include "../../lib/crypto/pkcs7_parser.h" > +#include <crypto/pkcs7_parser.h> > #else > #ifdef CONFIG_X509_CERTIFICATE_PARSER > -#include "../../lib/crypto/x509_parser.h" > +#include <crypto/x509_parser.h> > #endif > #endif > >
Heinrich, On Tue, Apr 21, 2020 at 12:26:08PM +0200, Heinrich Schuchardt wrote: > On 4/21/20 2:38 AM, AKASHI Takahiro wrote: > > Pkcs7_parse.h and x509_parser.h are used in UEFI subsystem, in particular, > > secure boot. So move them to include/crypto to avoid relative paths. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > > Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > > --- > > {lib => include}/crypto/pkcs7_parser.h | 0 > > {lib => include}/crypto/x509_parser.h | 0 > > lib/crypto/pkcs7_parser.c | 4 ++++ > > lib/crypto/x509_cert_parser.c | 4 ++++ > > lib/crypto/x509_public_key.c | 6 ++++-- > > lib/efi_loader/efi_image_loader.c | 4 ++-- > > lib/efi_loader/efi_signature.c | 4 ++-- > > lib/efi_loader/efi_variable.c | 2 +- > > test/lib/asn1.c | 4 ++-- > > 9 files changed, 19 insertions(+), 9 deletions(-) > > rename {lib => include}/crypto/pkcs7_parser.h (100%) > > rename {lib => include}/crypto/x509_parser.h (100%) > > > > diff --git a/lib/crypto/pkcs7_parser.h b/include/crypto/pkcs7_parser.h > > similarity index 100% > > rename from lib/crypto/pkcs7_parser.h > > rename to include/crypto/pkcs7_parser.h > > diff --git a/lib/crypto/x509_parser.h b/include/crypto/x509_parser.h > > similarity index 100% > > rename from lib/crypto/x509_parser.h > > rename to include/crypto/x509_parser.h > > diff --git a/lib/crypto/pkcs7_parser.c b/lib/crypto/pkcs7_parser.c > > index f5dda1179f8a..0ee207b6b1c8 100644 > > --- a/lib/crypto/pkcs7_parser.c > > +++ b/lib/crypto/pkcs7_parser.c > > @@ -20,7 +20,11 @@ > > #include <linux/err.h> > > #include <linux/oid_registry.h> > > #include <crypto/public_key.h> > > +#ifdef __UBOOT__ > > +#include <crypto/pkcs7_parser.h> > > +#else > > #include "pkcs7_parser.h" > > +#endif > > #include "pkcs7.asn1.h" > > > > MODULE_DESCRIPTION("PKCS#7 parser"); > > diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c > > index 4e41cffd2301..18f5407a076c 100644 > > --- a/lib/crypto/x509_cert_parser.c > > +++ b/lib/crypto/x509_cert_parser.c > > @@ -18,7 +18,11 @@ > > #include <linux/string.h> > > #endif > > #include <crypto/public_key.h> > > +#ifdef __UBOOT__ > > +#include <crypto/x509_parser.h> > > +#else > > #include "x509_parser.h" > > +#endif > > #include "x509.asn1.h" > > #include "x509_akid.asn1.h" > > > > diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c > > index 676c0df17410..571af9a0adf9 100644 > > --- a/lib/crypto/x509_public_key.c > > +++ b/lib/crypto/x509_public_key.c > > @@ -16,15 +16,17 @@ > > #include <linux/module.h> > > #endif > > #include <linux/kernel.h> > > -#ifndef __UBOOT__ > > +#ifdef __UBOOT__ > > +#include <crypto/x509_parser.h> > > +#else > > #include <linux/slab.h> > > #include <keys/asymmetric-subtype.h> > > #include <keys/asymmetric-parser.h> > > #include <keys/system_keyring.h> > > #include <crypto/hash.h> > > #include "asymmetric_keys.h" > > -#endif > > #include "x509_parser.h" > > +#endif > > > > /* > > * Set up the signature parameters in an X.509 certificate. This involves > > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > > index 899adf8493d0..f59b9a01140e 100644 > > --- a/lib/efi_loader/efi_image_loader.c > > +++ b/lib/efi_loader/efi_image_loader.c > > @@ -13,8 +13,8 @@ > > #include <malloc.h> > > #include <pe.h> > > #include <sort.h> > > -#include "../lib/crypto/pkcs7_parser.h" > > -#include "../lib/crypto/x509_parser.h" > > +#include "crypto/pkcs7_parser.h" > > +#include "crypto/x509_parser.h" > > Thanks for fixing this. > > x509_parser.h is included in pkcs7_parser.h. Please, remove the > superfluous line. See my comment in [1] [1] https://lists.denx.de/pipermail/u-boot/2020-April/408160.html -Takahiro Akashi > > > > const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; > > const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID; > > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c > > index 6ad09e4acbd7..0ead10203aeb 100644 > > --- a/lib/efi_loader/efi_signature.c > > +++ b/lib/efi_loader/efi_signature.c > > @@ -10,12 +10,12 @@ > > #include <image.h> > > #include <hexdump.h> > > #include <malloc.h> > > +#include <crypto/pkcs7_parser.h> > > +#include <crypto/x509_parser.h> > > Same here. > > Best regards > > Heinrich > > > #include <linux/compat.h> > > #include <linux/oid_registry.h> > > #include <u-boot/rsa.h> > > #include <u-boot/sha256.h> > > -#include "../lib/crypto/pkcs7_parser.h" > > -#include "../lib/crypto/x509_parser.h" > > > > const efi_guid_t efi_guid_image_security_database = > > EFI_IMAGE_SECURITY_DATABASE_GUID; > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > > index 7df881a74b44..0c6d1deb58eb 100644 > > --- a/lib/efi_loader/efi_variable.c > > +++ b/lib/efi_loader/efi_variable.c > > @@ -12,9 +12,9 @@ > > #include <malloc.h> > > #include <rtc.h> > > #include <search.h> > > +#include <crypto/pkcs7_parser.h> > > #include <linux/compat.h> > > #include <u-boot/crc.h> > > -#include "../lib/crypto/pkcs7_parser.h" > > > > enum efi_secure_mode { > > EFI_MODE_SETUP, > > diff --git a/test/lib/asn1.c b/test/lib/asn1.c > > index d2b3f67e68da..8661fdd30687 100644 > > --- a/test/lib/asn1.c > > +++ b/test/lib/asn1.c > > @@ -13,10 +13,10 @@ > > #include <test/ut.h> > > > > #ifdef CONFIG_PKCS7_MESSAGE_PARSER > > -#include "../../lib/crypto/pkcs7_parser.h" > > +#include <crypto/pkcs7_parser.h> > > #else > > #ifdef CONFIG_X509_CERTIFICATE_PARSER > > -#include "../../lib/crypto/x509_parser.h" > > +#include <crypto/x509_parser.h> > > #endif > > #endif > > > > >
On 23.04.20 02:31, AKASHI Takahiro wrote: > Heinrich, > > On Tue, Apr 21, 2020 at 12:26:08PM +0200, Heinrich Schuchardt wrote: >> On 4/21/20 2:38 AM, AKASHI Takahiro wrote: >>> Pkcs7_parse.h and x509_parser.h are used in UEFI subsystem, in particular, >>> secure boot. So move them to include/crypto to avoid relative paths. >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> >>> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >>> --- >>> {lib => include}/crypto/pkcs7_parser.h | 0 >>> {lib => include}/crypto/x509_parser.h | 0 >>> lib/crypto/pkcs7_parser.c | 4 ++++ >>> lib/crypto/x509_cert_parser.c | 4 ++++ >>> lib/crypto/x509_public_key.c | 6 ++++-- >>> lib/efi_loader/efi_image_loader.c | 4 ++-- >>> lib/efi_loader/efi_signature.c | 4 ++-- >>> lib/efi_loader/efi_variable.c | 2 +- >>> test/lib/asn1.c | 4 ++-- >>> 9 files changed, 19 insertions(+), 9 deletions(-) >>> rename {lib => include}/crypto/pkcs7_parser.h (100%) >>> rename {lib => include}/crypto/x509_parser.h (100%) >>> >>> diff --git a/lib/crypto/pkcs7_parser.h b/include/crypto/pkcs7_parser.h >>> similarity index 100% >>> rename from lib/crypto/pkcs7_parser.h >>> rename to include/crypto/pkcs7_parser.h >>> diff --git a/lib/crypto/x509_parser.h b/include/crypto/x509_parser.h >>> similarity index 100% >>> rename from lib/crypto/x509_parser.h >>> rename to include/crypto/x509_parser.h >>> diff --git a/lib/crypto/pkcs7_parser.c b/lib/crypto/pkcs7_parser.c >>> index f5dda1179f8a..0ee207b6b1c8 100644 >>> --- a/lib/crypto/pkcs7_parser.c >>> +++ b/lib/crypto/pkcs7_parser.c >>> @@ -20,7 +20,11 @@ >>> #include <linux/err.h> >>> #include <linux/oid_registry.h> >>> #include <crypto/public_key.h> >>> +#ifdef __UBOOT__ >>> +#include <crypto/pkcs7_parser.h> >>> +#else >>> #include "pkcs7_parser.h" >>> +#endif >>> #include "pkcs7.asn1.h" >>> >>> MODULE_DESCRIPTION("PKCS#7 parser"); >>> diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c >>> index 4e41cffd2301..18f5407a076c 100644 >>> --- a/lib/crypto/x509_cert_parser.c >>> +++ b/lib/crypto/x509_cert_parser.c >>> @@ -18,7 +18,11 @@ >>> #include <linux/string.h> >>> #endif >>> #include <crypto/public_key.h> >>> +#ifdef __UBOOT__ >>> +#include <crypto/x509_parser.h> >>> +#else >>> #include "x509_parser.h" >>> +#endif >>> #include "x509.asn1.h" >>> #include "x509_akid.asn1.h" >>> >>> diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c >>> index 676c0df17410..571af9a0adf9 100644 >>> --- a/lib/crypto/x509_public_key.c >>> +++ b/lib/crypto/x509_public_key.c >>> @@ -16,15 +16,17 @@ >>> #include <linux/module.h> >>> #endif >>> #include <linux/kernel.h> >>> -#ifndef __UBOOT__ >>> +#ifdef __UBOOT__ >>> +#include <crypto/x509_parser.h> >>> +#else >>> #include <linux/slab.h> >>> #include <keys/asymmetric-subtype.h> >>> #include <keys/asymmetric-parser.h> >>> #include <keys/system_keyring.h> >>> #include <crypto/hash.h> >>> #include "asymmetric_keys.h" >>> -#endif >>> #include "x509_parser.h" >>> +#endif >>> >>> /* >>> * Set up the signature parameters in an X.509 certificate. This involves >>> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c >>> index 899adf8493d0..f59b9a01140e 100644 >>> --- a/lib/efi_loader/efi_image_loader.c >>> +++ b/lib/efi_loader/efi_image_loader.c >>> @@ -13,8 +13,8 @@ >>> #include <malloc.h> >>> #include <pe.h> >>> #include <sort.h> >>> -#include "../lib/crypto/pkcs7_parser.h" >>> -#include "../lib/crypto/x509_parser.h" >>> +#include "crypto/pkcs7_parser.h" >>> +#include "crypto/x509_parser.h" >> >> Thanks for fixing this. >> >> x509_parser.h is included in pkcs7_parser.h. Please, remove the >> superfluous line. > > See my comment in [1] > > [1] https://lists.denx.de/pipermail/u-boot/2020-April/408160.html We want U-Boot to build as fast as possible. So we should not include anything twice. When looking through the rest of the U-Boot code you will have noticed that we do not include malloc.h and stdio.h everywhere because it is included via common.h. As an example of a patch removing other unnecessary includes see for instance: cmd: fat: remove unused includes fd0e30b43b6b2401e68dc32c357869c617d4fdd1 Best regards Heinrich > > -Takahiro Akashi > > >>> >>> const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; >>> const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID; >>> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c >>> index 6ad09e4acbd7..0ead10203aeb 100644 >>> --- a/lib/efi_loader/efi_signature.c >>> +++ b/lib/efi_loader/efi_signature.c >>> @@ -10,12 +10,12 @@ >>> #include <image.h> >>> #include <hexdump.h> >>> #include <malloc.h> >>> +#include <crypto/pkcs7_parser.h> >>> +#include <crypto/x509_parser.h> >> >> Same here. >> >> Best regards >> >> Heinrich >> >>> #include <linux/compat.h> >>> #include <linux/oid_registry.h> >>> #include <u-boot/rsa.h> >>> #include <u-boot/sha256.h> >>> -#include "../lib/crypto/pkcs7_parser.h" >>> -#include "../lib/crypto/x509_parser.h" >>> >>> const efi_guid_t efi_guid_image_security_database = >>> EFI_IMAGE_SECURITY_DATABASE_GUID; >>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c >>> index 7df881a74b44..0c6d1deb58eb 100644 >>> --- a/lib/efi_loader/efi_variable.c >>> +++ b/lib/efi_loader/efi_variable.c >>> @@ -12,9 +12,9 @@ >>> #include <malloc.h> >>> #include <rtc.h> >>> #include <search.h> >>> +#include <crypto/pkcs7_parser.h> >>> #include <linux/compat.h> >>> #include <u-boot/crc.h> >>> -#include "../lib/crypto/pkcs7_parser.h" >>> >>> enum efi_secure_mode { >>> EFI_MODE_SETUP, >>> diff --git a/test/lib/asn1.c b/test/lib/asn1.c >>> index d2b3f67e68da..8661fdd30687 100644 >>> --- a/test/lib/asn1.c >>> +++ b/test/lib/asn1.c >>> @@ -13,10 +13,10 @@ >>> #include <test/ut.h> >>> >>> #ifdef CONFIG_PKCS7_MESSAGE_PARSER >>> -#include "../../lib/crypto/pkcs7_parser.h" >>> +#include <crypto/pkcs7_parser.h> >>> #else >>> #ifdef CONFIG_X509_CERTIFICATE_PARSER >>> -#include "../../lib/crypto/x509_parser.h" >>> +#include <crypto/x509_parser.h> >>> #endif >>> #endif >>> >>> >>
Heinrich, On Thu, Apr 23, 2020 at 09:33:37AM +0200, Heinrich Schuchardt wrote: > On 23.04.20 02:31, AKASHI Takahiro wrote: > > Heinrich, > > > > On Tue, Apr 21, 2020 at 12:26:08PM +0200, Heinrich Schuchardt wrote: > >> On 4/21/20 2:38 AM, AKASHI Takahiro wrote: > >>> Pkcs7_parse.h and x509_parser.h are used in UEFI subsystem, in particular, > >>> secure boot. So move them to include/crypto to avoid relative paths. > >>> > >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> > >>> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > >>> --- > >>> {lib => include}/crypto/pkcs7_parser.h | 0 > >>> {lib => include}/crypto/x509_parser.h | 0 > >>> lib/crypto/pkcs7_parser.c | 4 ++++ > >>> lib/crypto/x509_cert_parser.c | 4 ++++ > >>> lib/crypto/x509_public_key.c | 6 ++++-- > >>> lib/efi_loader/efi_image_loader.c | 4 ++-- > >>> lib/efi_loader/efi_signature.c | 4 ++-- > >>> lib/efi_loader/efi_variable.c | 2 +- > >>> test/lib/asn1.c | 4 ++-- > >>> 9 files changed, 19 insertions(+), 9 deletions(-) > >>> rename {lib => include}/crypto/pkcs7_parser.h (100%) > >>> rename {lib => include}/crypto/x509_parser.h (100%) > >>> > >>> diff --git a/lib/crypto/pkcs7_parser.h b/include/crypto/pkcs7_parser.h > >>> similarity index 100% > >>> rename from lib/crypto/pkcs7_parser.h > >>> rename to include/crypto/pkcs7_parser.h > >>> diff --git a/lib/crypto/x509_parser.h b/include/crypto/x509_parser.h > >>> similarity index 100% > >>> rename from lib/crypto/x509_parser.h > >>> rename to include/crypto/x509_parser.h > >>> diff --git a/lib/crypto/pkcs7_parser.c b/lib/crypto/pkcs7_parser.c > >>> index f5dda1179f8a..0ee207b6b1c8 100644 > >>> --- a/lib/crypto/pkcs7_parser.c > >>> +++ b/lib/crypto/pkcs7_parser.c > >>> @@ -20,7 +20,11 @@ > >>> #include <linux/err.h> > >>> #include <linux/oid_registry.h> > >>> #include <crypto/public_key.h> > >>> +#ifdef __UBOOT__ > >>> +#include <crypto/pkcs7_parser.h> > >>> +#else > >>> #include "pkcs7_parser.h" > >>> +#endif > >>> #include "pkcs7.asn1.h" > >>> > >>> MODULE_DESCRIPTION("PKCS#7 parser"); > >>> diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c > >>> index 4e41cffd2301..18f5407a076c 100644 > >>> --- a/lib/crypto/x509_cert_parser.c > >>> +++ b/lib/crypto/x509_cert_parser.c > >>> @@ -18,7 +18,11 @@ > >>> #include <linux/string.h> > >>> #endif > >>> #include <crypto/public_key.h> > >>> +#ifdef __UBOOT__ > >>> +#include <crypto/x509_parser.h> > >>> +#else > >>> #include "x509_parser.h" > >>> +#endif > >>> #include "x509.asn1.h" > >>> #include "x509_akid.asn1.h" > >>> > >>> diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c > >>> index 676c0df17410..571af9a0adf9 100644 > >>> --- a/lib/crypto/x509_public_key.c > >>> +++ b/lib/crypto/x509_public_key.c > >>> @@ -16,15 +16,17 @@ > >>> #include <linux/module.h> > >>> #endif > >>> #include <linux/kernel.h> > >>> -#ifndef __UBOOT__ > >>> +#ifdef __UBOOT__ > >>> +#include <crypto/x509_parser.h> > >>> +#else > >>> #include <linux/slab.h> > >>> #include <keys/asymmetric-subtype.h> > >>> #include <keys/asymmetric-parser.h> > >>> #include <keys/system_keyring.h> > >>> #include <crypto/hash.h> > >>> #include "asymmetric_keys.h" > >>> -#endif > >>> #include "x509_parser.h" > >>> +#endif > >>> > >>> /* > >>> * Set up the signature parameters in an X.509 certificate. This involves > >>> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > >>> index 899adf8493d0..f59b9a01140e 100644 > >>> --- a/lib/efi_loader/efi_image_loader.c > >>> +++ b/lib/efi_loader/efi_image_loader.c > >>> @@ -13,8 +13,8 @@ > >>> #include <malloc.h> > >>> #include <pe.h> > >>> #include <sort.h> > >>> -#include "../lib/crypto/pkcs7_parser.h" > >>> -#include "../lib/crypto/x509_parser.h" > >>> +#include "crypto/pkcs7_parser.h" > >>> +#include "crypto/x509_parser.h" > >> > >> Thanks for fixing this. > >> > >> x509_parser.h is included in pkcs7_parser.h. Please, remove the > >> superfluous line. > > > > See my comment in [1] > > > > [1] https://lists.denx.de/pipermail/u-boot/2020-April/408160.html > > We want U-Boot to build as fast as possible. So we should not include > anything twice. When looking through the rest of the U-Boot code you > will have noticed that we do not include malloc.h and stdio.h everywhere > because it is included via common.h. I can't agree with you. "common.h" is not a good example; its purpose is to aggregate some some useful header files so that people don't get bothered with finding out an exact set of header files needed. "common.h" is even a *bad* example if you say, "We want U-Boot to build as fast as possible." because "common.h" may contain a lot of header files that may never be used in a given C file. So I won't change my patch. -Takahiro Akashi > As an example of a patch removing other unnecessary includes see for > instance: > > cmd: fat: remove unused includes > fd0e30b43b6b2401e68dc32c357869c617d4fdd1 > > Best regards > > Heinrich > > > > > -Takahiro Akashi > > > > > >>> > >>> const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; > >>> const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID; > >>> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c > >>> index 6ad09e4acbd7..0ead10203aeb 100644 > >>> --- a/lib/efi_loader/efi_signature.c > >>> +++ b/lib/efi_loader/efi_signature.c > >>> @@ -10,12 +10,12 @@ > >>> #include <image.h> > >>> #include <hexdump.h> > >>> #include <malloc.h> > >>> +#include <crypto/pkcs7_parser.h> > >>> +#include <crypto/x509_parser.h> > >> > >> Same here. > >> > >> Best regards > >> > >> Heinrich > >> > >>> #include <linux/compat.h> > >>> #include <linux/oid_registry.h> > >>> #include <u-boot/rsa.h> > >>> #include <u-boot/sha256.h> > >>> -#include "../lib/crypto/pkcs7_parser.h" > >>> -#include "../lib/crypto/x509_parser.h" > >>> > >>> const efi_guid_t efi_guid_image_security_database = > >>> EFI_IMAGE_SECURITY_DATABASE_GUID; > >>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > >>> index 7df881a74b44..0c6d1deb58eb 100644 > >>> --- a/lib/efi_loader/efi_variable.c > >>> +++ b/lib/efi_loader/efi_variable.c > >>> @@ -12,9 +12,9 @@ > >>> #include <malloc.h> > >>> #include <rtc.h> > >>> #include <search.h> > >>> +#include <crypto/pkcs7_parser.h> > >>> #include <linux/compat.h> > >>> #include <u-boot/crc.h> > >>> -#include "../lib/crypto/pkcs7_parser.h" > >>> > >>> enum efi_secure_mode { > >>> EFI_MODE_SETUP, > >>> diff --git a/test/lib/asn1.c b/test/lib/asn1.c > >>> index d2b3f67e68da..8661fdd30687 100644 > >>> --- a/test/lib/asn1.c > >>> +++ b/test/lib/asn1.c > >>> @@ -13,10 +13,10 @@ > >>> #include <test/ut.h> > >>> > >>> #ifdef CONFIG_PKCS7_MESSAGE_PARSER > >>> -#include "../../lib/crypto/pkcs7_parser.h" > >>> +#include <crypto/pkcs7_parser.h> > >>> #else > >>> #ifdef CONFIG_X509_CERTIFICATE_PARSER > >>> -#include "../../lib/crypto/x509_parser.h" > >>> +#include <crypto/x509_parser.h> > >>> #endif > >>> #endif > >>> > >>> > >> >
On 4/24/20 5:45 AM, AKASHI Takahiro wrote: > Heinrich, > > On Thu, Apr 23, 2020 at 09:33:37AM +0200, Heinrich Schuchardt wrote: >> On 23.04.20 02:31, AKASHI Takahiro wrote: >>> Heinrich, >>> >>> On Tue, Apr 21, 2020 at 12:26:08PM +0200, Heinrich Schuchardt wrote: >>>> On 4/21/20 2:38 AM, AKASHI Takahiro wrote: >>>>> Pkcs7_parse.h and x509_parser.h are used in UEFI subsystem, in particular, >>>>> secure boot. So move them to include/crypto to avoid relative paths. >>>>> >>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> >>>>> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> Patch is merged. Thanks Heinrich
diff --git a/lib/crypto/pkcs7_parser.h b/include/crypto/pkcs7_parser.h similarity index 100% rename from lib/crypto/pkcs7_parser.h rename to include/crypto/pkcs7_parser.h diff --git a/lib/crypto/x509_parser.h b/include/crypto/x509_parser.h similarity index 100% rename from lib/crypto/x509_parser.h rename to include/crypto/x509_parser.h diff --git a/lib/crypto/pkcs7_parser.c b/lib/crypto/pkcs7_parser.c index f5dda1179f8a..0ee207b6b1c8 100644 --- a/lib/crypto/pkcs7_parser.c +++ b/lib/crypto/pkcs7_parser.c @@ -20,7 +20,11 @@ #include <linux/err.h> #include <linux/oid_registry.h> #include <crypto/public_key.h> +#ifdef __UBOOT__ +#include <crypto/pkcs7_parser.h> +#else #include "pkcs7_parser.h" +#endif #include "pkcs7.asn1.h" MODULE_DESCRIPTION("PKCS#7 parser"); diff --git a/lib/crypto/x509_cert_parser.c b/lib/crypto/x509_cert_parser.c index 4e41cffd2301..18f5407a076c 100644 --- a/lib/crypto/x509_cert_parser.c +++ b/lib/crypto/x509_cert_parser.c @@ -18,7 +18,11 @@ #include <linux/string.h> #endif #include <crypto/public_key.h> +#ifdef __UBOOT__ +#include <crypto/x509_parser.h> +#else #include "x509_parser.h" +#endif #include "x509.asn1.h" #include "x509_akid.asn1.h" diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c index 676c0df17410..571af9a0adf9 100644 --- a/lib/crypto/x509_public_key.c +++ b/lib/crypto/x509_public_key.c @@ -16,15 +16,17 @@ #include <linux/module.h> #endif #include <linux/kernel.h> -#ifndef __UBOOT__ +#ifdef __UBOOT__ +#include <crypto/x509_parser.h> +#else #include <linux/slab.h> #include <keys/asymmetric-subtype.h> #include <keys/asymmetric-parser.h> #include <keys/system_keyring.h> #include <crypto/hash.h> #include "asymmetric_keys.h" -#endif #include "x509_parser.h" +#endif /* * Set up the signature parameters in an X.509 certificate. This involves diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 899adf8493d0..f59b9a01140e 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -13,8 +13,8 @@ #include <malloc.h> #include <pe.h> #include <sort.h> -#include "../lib/crypto/pkcs7_parser.h" -#include "../lib/crypto/x509_parser.h" +#include "crypto/pkcs7_parser.h" +#include "crypto/x509_parser.h" const efi_guid_t efi_global_variable_guid = EFI_GLOBAL_VARIABLE_GUID; const efi_guid_t efi_guid_device_path = EFI_DEVICE_PATH_PROTOCOL_GUID; diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c index 6ad09e4acbd7..0ead10203aeb 100644 --- a/lib/efi_loader/efi_signature.c +++ b/lib/efi_loader/efi_signature.c @@ -10,12 +10,12 @@ #include <image.h> #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> #include <u-boot/sha256.h> -#include "../lib/crypto/pkcs7_parser.h" -#include "../lib/crypto/x509_parser.h" const efi_guid_t efi_guid_image_security_database = EFI_IMAGE_SECURITY_DATABASE_GUID; diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 7df881a74b44..0c6d1deb58eb 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -12,9 +12,9 @@ #include <malloc.h> #include <rtc.h> #include <search.h> +#include <crypto/pkcs7_parser.h> #include <linux/compat.h> #include <u-boot/crc.h> -#include "../lib/crypto/pkcs7_parser.h" enum efi_secure_mode { EFI_MODE_SETUP, diff --git a/test/lib/asn1.c b/test/lib/asn1.c index d2b3f67e68da..8661fdd30687 100644 --- a/test/lib/asn1.c +++ b/test/lib/asn1.c @@ -13,10 +13,10 @@ #include <test/ut.h> #ifdef CONFIG_PKCS7_MESSAGE_PARSER -#include "../../lib/crypto/pkcs7_parser.h" +#include <crypto/pkcs7_parser.h> #else #ifdef CONFIG_X509_CERTIFICATE_PARSER -#include "../../lib/crypto/x509_parser.h" +#include <crypto/x509_parser.h> #endif #endif
Pkcs7_parse.h and x509_parser.h are used in UEFI subsystem, in particular, secure boot. So move them to include/crypto to avoid relative paths. Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de> --- {lib => include}/crypto/pkcs7_parser.h | 0 {lib => include}/crypto/x509_parser.h | 0 lib/crypto/pkcs7_parser.c | 4 ++++ lib/crypto/x509_cert_parser.c | 4 ++++ lib/crypto/x509_public_key.c | 6 ++++-- lib/efi_loader/efi_image_loader.c | 4 ++-- lib/efi_loader/efi_signature.c | 4 ++-- lib/efi_loader/efi_variable.c | 2 +- test/lib/asn1.c | 4 ++-- 9 files changed, 19 insertions(+), 9 deletions(-) rename {lib => include}/crypto/pkcs7_parser.h (100%) rename {lib => include}/crypto/x509_parser.h (100%)