| Message ID | 1504271303-1782-8-git-send-email-mw@semihalf.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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
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
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
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);