On 03/06/20 08:38, Ard Biesheuvel wrote:
> Bob reports that VS2017 chokes on a tentative definition of the const
> object 'mEfiFileProtocolTemplate', with the following error:
>
> OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
> error C2220: warning treated as error - no 'object' file generated
> OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
> warning C4132: 'mEfiFileProtocolTemplate': const object should be
> initialized
>
> Let's turn the only function that relies on this tentative definition
> into a forward declaration itself, and move its definition after the
> normal definition of the object. That allows us to drop the tentative
(1) s/normal/external/
> definition of the const object, and hopefully make VS2017 happy.
>
> Cc: "Feng, Bob C" <bob.c.f...@intel.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
> OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 134
> ++++++++++----------
> 1 file changed, 70 insertions(+), 64 deletions(-)
I agree that this is a bug in VS2017.
I've re-checked "6.9.2 External object definitions" in C99 now, and the
original code is correct. So is the commit message on the present patch.
>
> diff --git a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> index 869549f164f0..fbcdf019bf56 100644
> --- a/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> +++ b/OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
> @@ -123,13 +123,6 @@ typedef struct {
> #define STUB_FILE_FROM_FILE(FilePointer) \
> CR (FilePointer, STUB_FILE, File, STUB_FILE_SIG)
>
> -//
> -// Tentative definition of the file protocol template. The initializer
> -// (external definition) will be provided later.
> -//
> -STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate;
> -
> -
> //
> // Protocol member functions for File.
> //
> @@ -181,65 +174,10 @@ StubFileOpen (
> IN CHAR16 *FileName,
> IN UINT64 OpenMode,
> IN UINT64 Attributes
> - )
> -{
> - CONST STUB_FILE *StubFile;
> - UINTN BlobType;
> - STUB_FILE *NewStubFile;
> -
> + );
> //
> - // We're read-only.
> + // Forward declaration.
> //
(2) I suggest replacing this comment (which follows the function
declaration) with a small insertion to the normal leading comment (which
is already there):
/**
Opens a new file relative to the source file's location.
(Forward declaration.) <--- this
@param[in] This ...
**/
With (1) clarified, and (2) optionally fixed (no need to repost):
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks!
Laszlo
> - switch (OpenMode) {
> - case EFI_FILE_MODE_READ:
> - break;
> -
> - case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE:
> - case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE:
> - return EFI_WRITE_PROTECTED;
> -
> - default:
> - return EFI_INVALID_PARAMETER;
> - }
> -
> - //
> - // Only the root directory supports opening files in it.
> - //
> - StubFile = STUB_FILE_FROM_FILE (This);
> - if (StubFile->BlobType != KernelBlobTypeMax) {
> - return EFI_UNSUPPORTED;
> - }
> -
> - //
> - // Locate the file.
> - //
> - for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> - if (StrCmp (FileName, mKernelBlob[BlobType].Name) == 0) {
> - break;
> - }
> - }
> - if (BlobType == KernelBlobTypeMax) {
> - return EFI_NOT_FOUND;
> - }
> -
> - //
> - // Found it.
> - //
> - NewStubFile = AllocatePool (sizeof *NewStubFile);
> - if (NewStubFile == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> - }
> -
> - NewStubFile->Signature = STUB_FILE_SIG;
> - NewStubFile->BlobType = (KERNEL_BLOB_TYPE)BlobType;
> - NewStubFile->Position = 0;
> - CopyMem (&NewStubFile->File, &mEfiFileProtocolTemplate,
> - sizeof mEfiFileProtocolTemplate);
> - *NewHandle = &NewStubFile->File;
> -
> - return EFI_SUCCESS;
> -}
> -
>
> /**
> Closes a specified file handle.
> @@ -797,6 +735,74 @@ STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate
> = {
> NULL // FlushEx, revision 2
> };
>
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +StubFileOpen (
> + IN EFI_FILE_PROTOCOL *This,
> + OUT EFI_FILE_PROTOCOL **NewHandle,
> + IN CHAR16 *FileName,
> + IN UINT64 OpenMode,
> + IN UINT64 Attributes
> + )
> +{
> + CONST STUB_FILE *StubFile;
> + UINTN BlobType;
> + STUB_FILE *NewStubFile;
> +
> + //
> + // We're read-only.
> + //
> + switch (OpenMode) {
> + case EFI_FILE_MODE_READ:
> + break;
> +
> + case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE:
> + case EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE:
> + return EFI_WRITE_PROTECTED;
> +
> + default:
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // Only the root directory supports opening files in it.
> + //
> + StubFile = STUB_FILE_FROM_FILE (This);
> + if (StubFile->BlobType != KernelBlobTypeMax) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> + // Locate the file.
> + //
> + for (BlobType = 0; BlobType < KernelBlobTypeMax; ++BlobType) {
> + if (StrCmp (FileName, mKernelBlob[BlobType].Name) == 0) {
> + break;
> + }
> + }
> + if (BlobType == KernelBlobTypeMax) {
> + return EFI_NOT_FOUND;
> + }
> +
> + //
> + // Found it.
> + //
> + NewStubFile = AllocatePool (sizeof *NewStubFile);
> + if (NewStubFile == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + NewStubFile->Signature = STUB_FILE_SIG;
> + NewStubFile->BlobType = (KERNEL_BLOB_TYPE)BlobType;
> + NewStubFile->Position = 0;
> + CopyMem (&NewStubFile->File, &mEfiFileProtocolTemplate,
> + sizeof mEfiFileProtocolTemplate);
> + *NewHandle = &NewStubFile->File;
> +
> + return EFI_SUCCESS;
> +}
> +
>
> //
> // Protocol member functions for SimpleFileSystem.
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55605): https://edk2.groups.io/g/devel/message/55605
Mute This Topic: https://groups.io/mt/71768124/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
