1. Patchew
  2. EDK2
  3. [edk2] [platforms: PATCH 00/11] Armada 70x0/80x0 SPI improvements
  4. View patch

[edk2] [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues

Marcin Wojtas posted 11 patches 7 years, 3 months ago
[edk2] [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues
Posted by Marcin Wojtas 7 years, 3 months ago 
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Fix casting and related issues to make this code build for 32-bit ARM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
index edb6986..0951734 100644
--- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
+++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
@@ -172,6 +172,7 @@ PrepareFirmwareImage (
   EFI_STATUS           Status;
   UINT64               OpenMode;
   UINTN                *Buffer;
+  UINT64               Size;
 
   // Parse string from commandline
   FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
@@ -195,11 +196,13 @@ PrepareFirmwareImage (
       return EFI_DEVICE_ERROR;
     }
 
-  Status = FileHandleGetSize (*FileHandle, FileSize);
+  Status = FileHandleGetSize (*FileHandle, &Size);
     if (EFI_ERROR (Status)) {
       Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
     }
 
+  *FileSize = (UINTN)Size;
+
   // Read Image header into buffer
   Buffer = AllocateZeroPool (*FileSize);
 
-- 
1.8.3.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues
Posted by Leif Lindholm 7 years, 3 months ago 
On Fri, Sep 01, 2017 at 03:08:19PM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Fix casting and related issues to make this code build for 32-bit ARM.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> index edb6986..0951734 100644
> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> @@ -172,6 +172,7 @@ PrepareFirmwareImage (
>    EFI_STATUS           Status;
>    UINT64               OpenMode;
>    UINTN                *Buffer;
> +  UINT64               Size;
>  
>    // Parse string from commandline
>    FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
> @@ -195,11 +196,13 @@ PrepareFirmwareImage (
>        return EFI_DEVICE_ERROR;
>      }
>  
> -  Status = FileHandleGetSize (*FileHandle, FileSize);
> +  Status = FileHandleGetSize (*FileHandle, &Size);
>      if (EFI_ERROR (Status)) {
>        Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
>      }
>  
> +  *FileSize = (UINTN)Size;
> +

Rather than juggling around with temporary variables, why not make
FileSize in ShellCommandRunFUpdate() UINT64 and update
PrepareFirmwareImage() prototype accordingly?

/
     Leif

>    // Read Image header into buffer
>    Buffer = AllocateZeroPool (*FileSize);
>  
> -- 
> 1.8.3.1
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues
Posted by Ard Biesheuvel 7 years, 3 months ago 
On 1 September 2017 at 15:54, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Fri, Sep 01, 2017 at 03:08:19PM +0200, Marcin Wojtas wrote:
>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> Fix casting and related issues to make this code build for 32-bit ARM.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> index edb6986..0951734 100644
>> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
>> @@ -172,6 +172,7 @@ PrepareFirmwareImage (
>>    EFI_STATUS           Status;
>>    UINT64               OpenMode;
>>    UINTN                *Buffer;
>> +  UINT64               Size;
>>
>>    // Parse string from commandline
>>    FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
>> @@ -195,11 +196,13 @@ PrepareFirmwareImage (
>>        return EFI_DEVICE_ERROR;
>>      }
>>
>> -  Status = FileHandleGetSize (*FileHandle, FileSize);
>> +  Status = FileHandleGetSize (*FileHandle, &Size);
>>      if (EFI_ERROR (Status)) {
>>        Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
>>      }
>>
>> +  *FileSize = (UINTN)Size;
>> +
>
> Rather than juggling around with temporary variables, why not make
> FileSize in ShellCommandRunFUpdate() UINT64 and update
> PrepareFirmwareImage() prototype accordingly?
>

I don't remember /exactly/, but I think it breaks in another place then.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH 07/11] Applications/FirmwareUpdate: Fix 32-bit issues
Posted by Leif Lindholm 7 years, 3 months ago 
On Fri, Sep 01, 2017 at 04:16:24PM +0100, Ard Biesheuvel wrote:
> On 1 September 2017 at 15:54, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Fri, Sep 01, 2017 at 03:08:19PM +0200, Marcin Wojtas wrote:
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> Fix casting and related issues to make this code build for 32-bit ARM.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> >> ---
> >>  Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> >> index edb6986..0951734 100644
> >> --- a/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> >> +++ b/Platform/Marvell/Applications/FirmwareUpdate/FUpdate.c
> >> @@ -172,6 +172,7 @@ PrepareFirmwareImage (
> >>    EFI_STATUS           Status;
> >>    UINT64               OpenMode;
> >>    UINTN                *Buffer;
> >> +  UINT64               Size;
> >>
> >>    // Parse string from commandline
> >>    FileStr = ShellCommandLineGetRawValue (CheckPackage, 1);
> >> @@ -195,11 +196,13 @@ PrepareFirmwareImage (
> >>        return EFI_DEVICE_ERROR;
> >>      }
> >>
> >> -  Status = FileHandleGetSize (*FileHandle, FileSize);
> >> +  Status = FileHandleGetSize (*FileHandle, &Size);
> >>      if (EFI_ERROR (Status)) {
> >>        Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING);
> >>      }
> >>
> >> +  *FileSize = (UINTN)Size;
> >> +
> >
> > Rather than juggling around with temporary variables, why not make
> > FileSize in ShellCommandRunFUpdate() UINT64 and update
> > PrepareFirmwareImage() prototype accordingly?
> 
> I don't remember /exactly/, but I think it breaks in another place then.

The only thing I see that _could_ be affected is
    Status = SpiFlashProtocol->Update (Slave, 0, FileSize, (UINT8 *)FileBuffer);

And I would prefer a (UINTN) cast there over temporary variable
shuffling in a subfunction.

/
    Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.