diff mbox series

[1/1] tools/mkeficapsule: correct printf codes

Message ID 20240814071422.54193-1-heinrich.schuchardt@canonical.com
State Superseded
Delegated to: Heinrich Schuchardt
Headers show
Series [1/1] tools/mkeficapsule: correct printf codes | expand

Commit Message

Heinrich Schuchardt Aug. 14, 2024, 7:14 a.m. UTC
uint64_t is defined as unsigned long long on 32bit ARM.
Convert uint64_t values to unsigned long long and use %llX for printing.

    tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
    tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
    type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
    {aka ‘long long unsigned int’} [-Wformat=]
    694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
        |                                                      ~~~~^
        |                                                          |
        |                                                          long unsigned int
        |                                                      %08llX

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 tools/mkeficapsule.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Sughosh Ganu Aug. 14, 2024, 11:04 a.m. UTC | #1
On Wed, 14 Aug 2024 at 12:44, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> uint64_t is defined as unsigned long long on 32bit ARM.
> Convert uint64_t values to unsigned long long and use %llX for printing.
>
>     tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
>     tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
>     type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
>     {aka ‘long long unsigned int’} [-Wformat=]
>     694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
>         |                                                      ~~~~^
>         |                                                          |
>         |                                                          long unsigned int
>         |                                                      %08llX
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  tools/mkeficapsule.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Acked-by: Sughosh Ganu <sughosh.ganu@linaro.org>

Interesting that the tools are being built on a 32 bit host :)

-sughosh

>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index f28008a0829..d346c217f09 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -691,8 +691,8 @@ static uint32_t dump_fmp_payload_header(
>  static void dump_capsule_auth_header(
>         struct efi_firmware_image_authentication *capsule_auth_hdr)
>  {
> -       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
> -              capsule_auth_hdr->monotonic_count);
> +       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
> +              (unsigned long long)capsule_auth_hdr->monotonic_count);
>         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
>                capsule_auth_hdr->auth_info.hdr.dwLength);
>         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
> @@ -724,10 +724,10 @@ static void dump_fmp_capsule_image_header(
>                image_hdr->update_image_size);
>         printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
>                image_hdr->update_vendor_code_size);
> -       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
> -              image_hdr->update_hardware_instance);
> -       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
> -              image_hdr->image_capsule_support);
> +       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
> +              (unsigned long long)image_hdr->update_hardware_instance);
> +       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
> +              (unsigned long long)image_hdr->image_capsule_support);
>
>         printf("--------\n");
>         if (image_hdr->image_capsule_support & CAPSULE_SUPPORT_AUTHENTICATION) {
> --
> 2.45.2
>
Ilias Apalodimas Aug. 14, 2024, 11:40 a.m. UTC | #2
Hi Heinrich,

On Wed, 14 Aug 2024 at 10:14, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> uint64_t is defined as unsigned long long on 32bit ARM.
> Convert uint64_t values to unsigned long long and use %llX for printing.

Why do we need to convert? uint64_t is a fixed width type

Thanks
/Ilias
>
>     tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
>     tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
>     type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
>     {aka ‘long long unsigned int’} [-Wformat=]
>     694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
>         |                                                      ~~~~^
>         |                                                          |
>         |                                                          long unsigned int
>         |                                                      %08llX
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  tools/mkeficapsule.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index f28008a0829..d346c217f09 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -691,8 +691,8 @@ static uint32_t dump_fmp_payload_header(
>  static void dump_capsule_auth_header(
>         struct efi_firmware_image_authentication *capsule_auth_hdr)
>  {
> -       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
> -              capsule_auth_hdr->monotonic_count);
> +       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
> +              (unsigned long long)capsule_auth_hdr->monotonic_count);
>         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
>                capsule_auth_hdr->auth_info.hdr.dwLength);
>         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
> @@ -724,10 +724,10 @@ static void dump_fmp_capsule_image_header(
>                image_hdr->update_image_size);
>         printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
>                image_hdr->update_vendor_code_size);
> -       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
> -              image_hdr->update_hardware_instance);
> -       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
> -              image_hdr->image_capsule_support);
> +       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
> +              (unsigned long long)image_hdr->update_hardware_instance);
> +       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
> +              (unsigned long long)image_hdr->image_capsule_support);
>
>         printf("--------\n");
>         if (image_hdr->image_capsule_support & CAPSULE_SUPPORT_AUTHENTICATION) {
> --
> 2.45.2
>
Sughosh Ganu Aug. 14, 2024, 11:49 a.m. UTC | #3
On Wed, 14 Aug 2024 at 17:10, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Heinrich,
>
> On Wed, 14 Aug 2024 at 10:14, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > uint64_t is defined as unsigned long long on 32bit ARM.
> > Convert uint64_t values to unsigned long long and use %llX for printing.
>
> Why do we need to convert? uint64_t is a fixed width type

We get warnings on a 64 bit host like such

tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
tools/mkeficapsule.c:694:67: warning: format ‘%llX’ expects argument
of type ‘long long unsigned int’, but argument 2 has type ‘uint64_t’
{aka ‘long unsigned int’} [-Wformat=]
  694 |         printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
      |                                                              ~~~~~^
      |                                                                   |
      |
   long long unsigned int
      |                                                              %08lX
  695 |                capsule_auth_hdr->monotonic_count);
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                |
      |                                uint64_t {aka long unsigned int}


-sughosh



>
> Thanks
> /Ilias
> >
> >     tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
> >     tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
> >     type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
> >     {aka ‘long long unsigned int’} [-Wformat=]
> >     694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
> >         |                                                      ~~~~^
> >         |                                                          |
> >         |                                                          long unsigned int
> >         |                                                      %08llX
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---
> >  tools/mkeficapsule.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > index f28008a0829..d346c217f09 100644
> > --- a/tools/mkeficapsule.c
> > +++ b/tools/mkeficapsule.c
> > @@ -691,8 +691,8 @@ static uint32_t dump_fmp_payload_header(
> >  static void dump_capsule_auth_header(
> >         struct efi_firmware_image_authentication *capsule_auth_hdr)
> >  {
> > -       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
> > -              capsule_auth_hdr->monotonic_count);
> > +       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
> > +              (unsigned long long)capsule_auth_hdr->monotonic_count);
> >         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
> >                capsule_auth_hdr->auth_info.hdr.dwLength);
> >         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
> > @@ -724,10 +724,10 @@ static void dump_fmp_capsule_image_header(
> >                image_hdr->update_image_size);
> >         printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
> >                image_hdr->update_vendor_code_size);
> > -       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
> > -              image_hdr->update_hardware_instance);
> > -       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
> > -              image_hdr->image_capsule_support);
> > +       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
> > +              (unsigned long long)image_hdr->update_hardware_instance);
> > +       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
> > +              (unsigned long long)image_hdr->image_capsule_support);
> >
> >         printf("--------\n");
> >         if (image_hdr->image_capsule_support & CAPSULE_SUPPORT_AUTHENTICATION) {
> > --
> > 2.45.2
> >
Heinrich Schuchardt Aug. 14, 2024, 12:21 p.m. UTC | #4
On 14.08.24 13:40, Ilias Apalodimas wrote:
> Hi Heinrich,
> 
> On Wed, 14 Aug 2024 at 10:14, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> uint64_t is defined as unsigned long long on 32bit ARM.
>> Convert uint64_t values to unsigned long long and use %llX for printing.
> 
> Why do we need to convert? uint64_t is a fixed width type

Thank you for reviewing.

Including inttypes.h and using PRIX64 is probably a cleaner solution.

Best regards

Heinrich


> 
> Thanks
> /Ilias
>>
>>      tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
>>      tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
>>      type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
>>      {aka ‘long long unsigned int’} [-Wformat=]
>>      694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
>>          |                                                      ~~~~^
>>          |                                                          |
>>          |                                                          long unsigned int
>>          |                                                      %08llX
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>   tools/mkeficapsule.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
>> index f28008a0829..d346c217f09 100644
>> --- a/tools/mkeficapsule.c
>> +++ b/tools/mkeficapsule.c
>> @@ -691,8 +691,8 @@ static uint32_t dump_fmp_payload_header(
>>   static void dump_capsule_auth_header(
>>          struct efi_firmware_image_authentication *capsule_auth_hdr)
>>   {
>> -       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
>> -              capsule_auth_hdr->monotonic_count);
>> +       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
>> +              (unsigned long long)capsule_auth_hdr->monotonic_count);
>>          printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
>>                 capsule_auth_hdr->auth_info.hdr.dwLength);
>>          printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
>> @@ -724,10 +724,10 @@ static void dump_fmp_capsule_image_header(
>>                 image_hdr->update_image_size);
>>          printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
>>                 image_hdr->update_vendor_code_size);
>> -       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
>> -              image_hdr->update_hardware_instance);
>> -       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
>> -              image_hdr->image_capsule_support);
>> +       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
>> +              (unsigned long long)image_hdr->update_hardware_instance);
>> +       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
>> +              (unsigned long long)image_hdr->image_capsule_support);
>>
>>          printf("--------\n");
>>          if (image_hdr->image_capsule_support & CAPSULE_SUPPORT_AUTHENTICATION) {
>> --
>> 2.45.2
>>
Simon Glass Aug. 14, 2024, 12:40 p.m. UTC | #5
Hi Heinrich,

On Wed, 14 Aug 2024 at 01:14, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> uint64_t is defined as unsigned long long on 32bit ARM.
> Convert uint64_t values to unsigned long long and use %llX for printing.
>
>     tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
>     tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
>     type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
>     {aka ‘long long unsigned int’} [-Wformat=]
>     694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
>         |                                                      ~~~~^
>         |                                                          |
>         |                                                          long unsigned int
>         |                                                      %08llX
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  tools/mkeficapsule.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index f28008a0829..d346c217f09 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -691,8 +691,8 @@ static uint32_t dump_fmp_payload_header(
>  static void dump_capsule_auth_header(
>         struct efi_firmware_image_authentication *capsule_auth_hdr)
>  {
> -       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
> -              capsule_auth_hdr->monotonic_count);
> +       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
> +              (unsigned long long)capsule_auth_hdr->monotonic_count);
>         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
>                capsule_auth_hdr->auth_info.hdr.dwLength);
>         printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
> @@ -724,10 +724,10 @@ static void dump_fmp_capsule_image_header(
>                image_hdr->update_image_size);
>         printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
>                image_hdr->update_vendor_code_size);
> -       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
> -              image_hdr->update_hardware_instance);
> -       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
> -              image_hdr->image_capsule_support);
> +       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
> +              (unsigned long long)image_hdr->update_hardware_instance);
> +       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
> +              (unsigned long long)image_hdr->image_capsule_support);

These should be in lower-case hex, by the way. Also see inttypes.h

>
>         printf("--------\n");
>         if (image_hdr->image_capsule_support & CAPSULE_SUPPORT_AUTHENTICATION) {
> --
> 2.45.2
>

Regards,
SImon
Heinrich Schuchardt Aug. 14, 2024, 12:59 p.m. UTC | #6
On 14.08.24 14:40, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 14 Aug 2024 at 01:14, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> uint64_t is defined as unsigned long long on 32bit ARM.
>> Convert uint64_t values to unsigned long long and use %llX for printing.
>>
>>      tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
>>      tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
>>      type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
>>      {aka ‘long long unsigned int’} [-Wformat=]
>>      694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
>>          |                                                      ~~~~^
>>          |                                                          |
>>          |                                                          long unsigned int
>>          |                                                      %08llX
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>   tools/mkeficapsule.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
>> index f28008a0829..d346c217f09 100644
>> --- a/tools/mkeficapsule.c
>> +++ b/tools/mkeficapsule.c
>> @@ -691,8 +691,8 @@ static uint32_t dump_fmp_payload_header(
>>   static void dump_capsule_auth_header(
>>          struct efi_firmware_image_authentication *capsule_auth_hdr)
>>   {
>> -       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
>> -              capsule_auth_hdr->monotonic_count);
>> +       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
>> +              (unsigned long long)capsule_auth_hdr->monotonic_count);
>>          printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
>>                 capsule_auth_hdr->auth_info.hdr.dwLength);
>>          printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
>> @@ -724,10 +724,10 @@ static void dump_fmp_capsule_image_header(
>>                 image_hdr->update_image_size);
>>          printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
>>                 image_hdr->update_vendor_code_size);
>> -       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
>> -              image_hdr->update_hardware_instance);
>> -       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
>> -              image_hdr->image_capsule_support);
>> +       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
>> +              (unsigned long long)image_hdr->update_hardware_instance);
>> +       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
>> +              (unsigned long long)image_hdr->image_capsule_support);
> 
> These should be in lower-case hex, by the way. Also see inttypes.h

Why would you print some lines in lower case while the rest is upper case?

I have sent a second version using PRIX64 and avoiding the conversion.

Best regards

Heinrich


> 
>>
>>          printf("--------\n");
>>          if (image_hdr->image_capsule_support & CAPSULE_SUPPORT_AUTHENTICATION) {
>> --
>> 2.45.2
>>
> 
> Regards,
> SImon
Simon Glass Aug. 15, 2024, 8:31 p.m. UTC | #7
Hi Heinrich,

On Wed, 14 Aug 2024 at 06:59, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 14.08.24 14:40, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Wed, 14 Aug 2024 at 01:14, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> uint64_t is defined as unsigned long long on 32bit ARM.
> >> Convert uint64_t values to unsigned long long and use %llX for printing.
> >>
> >>      tools/mkeficapsule.c: In function ‘dump_capsule_auth_header’:
> >>      tools/mkeficapsule.c:694:66: warning: format ‘%lX’ expects argument of
> >>      type ‘long unsigned int’, but argument 2 has type ‘uint64_t’
> >>      {aka ‘long long unsigned int’} [-Wformat=]
> >>      694 | printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
> >>          |                                                      ~~~~^
> >>          |                                                          |
> >>          |                                                          long unsigned int
> >>          |                                                      %08llX
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >>   tools/mkeficapsule.c | 12 ++++++------
> >>   1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> >> index f28008a0829..d346c217f09 100644
> >> --- a/tools/mkeficapsule.c
> >> +++ b/tools/mkeficapsule.c
> >> @@ -691,8 +691,8 @@ static uint32_t dump_fmp_payload_header(
> >>   static void dump_capsule_auth_header(
> >>          struct efi_firmware_image_authentication *capsule_auth_hdr)
> >>   {
> >> -       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
> >> -              capsule_auth_hdr->monotonic_count);
> >> +       printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
> >> +              (unsigned long long)capsule_auth_hdr->monotonic_count);
> >>          printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
> >>                 capsule_auth_hdr->auth_info.hdr.dwLength);
> >>          printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
> >> @@ -724,10 +724,10 @@ static void dump_fmp_capsule_image_header(
> >>                 image_hdr->update_image_size);
> >>          printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
> >>                 image_hdr->update_vendor_code_size);
> >> -       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
> >> -              image_hdr->update_hardware_instance);
> >> -       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
> >> -              image_hdr->image_capsule_support);
> >> +       printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
> >> +              (unsigned long long)image_hdr->update_hardware_instance);
> >> +       printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
> >> +              (unsigned long long)image_hdr->image_capsule_support);
> >
> > These should be in lower-case hex, by the way. Also see inttypes.h
>
> Why would you print some lines in lower case while the rest is upper case?
>
> I have sent a second version using PRIX64 and avoiding the conversion.

Please send a second patch to convert them all to lower case.

- Simon
diff mbox series

Patch

diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index f28008a0829..d346c217f09 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -691,8 +691,8 @@  static uint32_t dump_fmp_payload_header(
 static void dump_capsule_auth_header(
 	struct efi_firmware_image_authentication *capsule_auth_hdr)
 {
-	printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08lX\n",
-	       capsule_auth_hdr->monotonic_count);
+	printf("EFI_FIRMWARE_IMAGE_AUTH.MONOTONIC_COUNT\t\t: %08llX\n",
+	       (unsigned long long)capsule_auth_hdr->monotonic_count);
 	printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.dwLENGTH\t: %08X\n",
 	       capsule_auth_hdr->auth_info.hdr.dwLength);
 	printf("EFI_FIRMWARE_IMAGE_AUTH.AUTH_INFO.HDR.wREVISION\t: %08X\n",
@@ -724,10 +724,10 @@  static void dump_fmp_capsule_image_header(
 	       image_hdr->update_image_size);
 	printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_VENDOR_CODE_SIZE\t: %08X\n",
 	       image_hdr->update_vendor_code_size);
-	printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08lX\n",
-	       image_hdr->update_hardware_instance);
-	printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08lX\n",
-	       image_hdr->image_capsule_support);
+	printf("FMP_CAPSULE_IMAGE_HDR.UPDATE_HARDWARE_INSTANCE\t: %08llX\n",
+	       (unsigned long long)image_hdr->update_hardware_instance);
+	printf("FMP_CAPSULE_IMAGE_HDR.IMAGE_CAPSULE_SUPPORT\t: %08llX\n",
+	       (unsigned long long)image_hdr->image_capsule_support);
 
 	printf("--------\n");
 	if (image_hdr->image_capsule_support & CAPSULE_SUPPORT_AUTHENTICATION) {