[PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Ard Biesheuvel]

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
definition of the const object, and hopefully make VS2017 happy.

Cc: "Feng, Bob C" <bob.c.feng@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 134 ++++++++++----------
 1 file changed, 70 insertions(+), 64 deletions(-)

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.
   //
-  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.
-- 
2.17.1

Re: [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Laszlo Ersek]

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.feng@intel.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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 <lersek@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.
> 

Re: [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Ard Biesheuvel]

On Fri, 6 Mar 2020 at 17:14, Laszlo Ersek <lersek@redhat.com> wrote:
>
> 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/
>

Are you sure? The const object has static linkage.

> > definition of the const object, and hopefully make VS2017 happy.
> >
> > Cc: "Feng, Bob C" <bob.c.feng@intel.com>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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 <lersek@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.
> >
>

Re: [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Laszlo Ersek]

On 03/06/20 17:40, Ard Biesheuvel wrote:
> On Fri, 6 Mar 2020 at 17:14, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> 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/
>>
>
> Are you sure? The const object has static linkage.

Yes, I'm sure.

- Linkage can be external, internal, or none.

- Storage duration can be static, automatic, or allocated.

- Type qualifiers are: const, restrict, and volatile.

Quoting ISO C99 "6.9 External definitions" (as background):

> Constraints
>
> 3 There shall be no more than one external definition for each
>   identifier declared with internal linkage in a translation unit.
>   Moreover, if an identifier declared with internal linkage is used in
>   an expression (other than as a part of the operand of a sizeof
>   operator whose result is an integer constant), there shall be
>   exactly one external definition for the identifier in the
>   translation unit.
>
> Semantics
>
> 4 As discussed in 5.1.1.1, the unit of program text after
>   preprocessing is a translation unit, which consists of a sequence of
>   external declarations. These are described as "external" because
>   they appear outside any function (and hence have file scope). As
>   discussed in 6.7, a declaration that also causes storage to be
>   reserved for an object or a function named by the identifier is a
>   definition.
>
> 5 An external definition is an external declaration that is also a
>   definition of a function (other than an inline definition) or an
>   object. If an identifier declared with external linkage is used in
>   an expression (other than as part of the operand of a sizeof
>   operator whose result is an integer constant), somewhere in the
>   entire program there shall be exactly one external definition for
>   the identifier; otherwise, there shall be no more than one. 140)
>
> Footnote 140: Thus, if an identifier declared with external linkage is
>               not used in an expression, there need be no external
>               definition for it.

Furthermore, from "6.9.2 External object definitions":

> Semantics
>
> 1 If the declaration of an identifier for an object has file scope and
>   an initializer, the declaration is an external definition for the
>   identifier.
>
> 2 A declaration of an identifier for an object that has file scope
>   without an initializer, and without a storage-class specifier or
>   with the storage-class specifier static, constitutes a tentative
>   definition. If a translation unit contains one or more tentative
>   definitions for an identifier, and the translation unit contains no
>   external definition for that identifier, then the behavior is
>   exactly as if the translation unit contains a file scope declaration
>   of that identifier, with the composite type as of the end of the
>   translation unit, with an initializer equal to 0.
>
> 3 If the declaration of an identifier for an object is a tentative
>   definition and has internal linkage, the declared type shall not be
>   an incomplete type.

In the original code, the first

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate;

is a tentative definition, per 6.9.2p2, because:

- it has file scope,
- it has no initializer,
- it has storage-class specifier "static".

Note that type qualifiers (such as "const") are not relevant for this.
(Which is why the error message from VS2017 is a bug.)

Paragraph 6.9.2p3 also applies:

- the linkage is internal [*],

- the type is *not* incomplete, so we do satisfy paragraph 3.

(

[*] The linkage is internal per "6.2.2 Linkages of identifiers":

> 3 If the declaration of a file scope identifier for an object [...]
>   contains the storage-class specifier static, the identifier has
>   internal linkage. [...]

)

And, finally, to answer your question,

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate = {...};

is an external declaration per 6.9.2p1:

- file scope: check,
- initializer: check,
- linkage: does not matter.

The confusion arises because "external" is used in two senses: once for
linkage, and another time for scope (file scope). But linkage and scope
are different concepts.


Unfortunately, these bits an pieces are quite scattered all over the
language standard. Which is why I had prepared the following handy table
several years -- more then a decade -- ago, as a reference for myself.

Note that the chapter and paragraph locations in the table are still
based on the C89 standard; I have never bothered to update them to C99.

Feel free to keep it for further reference, if you feel you can trust it
:)

The first two columns constitute the "input", i.e., they let you match a
declaration "pattern", and the scope (file or block) where the
declaration occurs.

Then the other three ("output") columns tell you what the linkage,
storage duration, and definition (if any) of the declaration are.

> Name space: ordinary identifiers
>
> Declaration             | Scope | Linkage                                                            | Storage duration       | Definition
> ------------------------+-------+--------------------------------------------------------------------+------------------------+--------------------------
>        int obj_1;       | file  | external ((6.1.2.2 p5)                                             | static (6.1.2.4 p2)    | tentative (6.7.2 p2)
> extern int obj_2;       | file  | same as any visible file scope declaration / external (6.1.2.2 p4) | static (6.1.2.4 p2)*   | none (6.7.2)
> static int obj_3;       | file  | internal (6.1.2.2 p3)                                              | static (6.1.2.4 p2)    | tentative (6.7.2 p2)
>        int obj_4 = 1;   | file  | external ((6.1.2.2 p5)                                             | static (6.1.2.4 p2)    | external (6.7.2 p1)
> extern int obj_5 = 1;   | file  | same as any visible file scope declaration / external (6.1.2.2 p4) | static (6.1.2.4 p2)*   | external (6.7.2 p1)
> static int obj_6 = 1;   | file  | internal (6.1.2.2 p3)                                              | static (6.1.2.4 p2)    | external (6.7.2 p1)
>        int fun_1(void); | file  | same as any visible file scope declaration / external (6.1.2.2 p5) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> extern int fun_2(void); | file  | same as any visible file scope declaration / external (6.1.2.2 p4) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> static int fun_3(void); | file  | internal (6.1.2.2 p3)                                              | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> ------------------------+-------+--------------------------------------------------------------------+------------------------+--------------------------
>        int obj_7;       | block | none (6.1.2.2 p6)                                                  | automatic (6.1.2.4 p3) | yes (6.1.2.4 p3, 6.5 p4)
> extern int obj_8;       | block | same as any visible file scope declaration / external (6.1.2.2 p4) | static (6.1.2.4 p2)*   | none (6.7.2)
> static int obj_9;       | block | none (6.1.2.2 p6)                                                  | static (6.1.2.4 p2)    | yes (6.1.2.4 p2, 6.5 p4)
>        int obj_10 = 1;  | block | none (6.1.2.2 p6)                                                  | automatic (6.1.2.4 p3) | yes (6.1.2.4 p3, 6.5 p4)
> extern int obj_11 = 1;  | block | INVALID (6.5.7 p4)**                                               | INVALID                | INVALID
> static int obj_12 = 1;  | block | none (6.1.2.2 p6)                                                  | static (6.1.2.4 p2)    | yes (6.1.2.4 p2, 6.5 p4)
>        int fun_4(void); | block | same as any visible file scope declaration / external (6.1.2.2 p5) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> extern int fun_5(void); | block | same as any visible file scope declaration / external (6.1.2.2 p4) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> static int fun_6(void); | block | INVALID (6.1.2.2 p3 fn13, 6.5.1 p4)                                | INVALID                | INVALID
>
> * any visible file scope declaration can only specify either external or internal linkage
>
> ** such a declaration would specify "same as any visible file scope declaration / external (6.1.2.2 p4)" linkage, that is, external or internal (see *)

Using this table,

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate;

is matched by "obj_3" in the table:

- declaration:
  static int obj_3;

- scope:
  file

and

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate = {...};

is matched by "obj_6" in the table:

- declaration:
  static int obj_6 = 1;

- scope:
  file

In both cases (obj_3, obj_6), we have internal linkage, and static
storage duration.

In the first case (obj_3), we have a tentative definition.

In the second case (obj_6), we have an external definition.

... Since we're talking cheat sheets, here's another one I like to refer
to, for operator binding and associativity (I typed this up in 2004):

> operator                                    associativity
> ---------------------------------------------------------
> ()  []  .  ->                               left to right
> !  ~  -  ++  --  &  *  (type)  sizeof       right to left
> *  /  %                                     left to right
> +  -                                        left to right
> <<  >>                                      left to right
> <  <=  >  >=                                left to right
> ==  !=                                      left to right
> &                                           left to right
> ^                                           left to right
> |                                           left to right
> &&                                          left to right
> ||                                          left to right
> ?:                                          right to left
> =  *=  /=  %=  +=  -=  <<= >>=  &=  ^=  |=  right to left
> ,                                           left to right

Operators near the top of the table bind more strongly.

Thanks,
Laszlo

Re: [edk2-devel] [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Ard Biesheuvel]

On Fri, 6 Mar 2020 at 20:22, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 03/06/20 17:40, Ard Biesheuvel wrote:
> > On Fri, 6 Mar 2020 at 17:14, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> 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/
> >>
> >
> > Are you sure? The const object has static linkage.
>
> Yes, I'm sure.
>

:-)

I should have known better than to doubt you on a matter like this ...

Thanks for the explanation.

Re: [edk2-devel] [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Bob Feng]

Hi Ard,

With this patch, Ovmf still build failed on windows with VS2017. The error message like below:

Building ... d:\edk2maintain\edk2\MdeModulePkg\Universal\Metronome\Metronome.inf [IA32]
d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.c(199): error C2220: warning treated as error - no 'object' file generated
d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.c(199): warning C4244: '=': conversion from 'UINT64' to 'UINTN', possible loss of data
        "C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.exe" /Fod:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\Bus\Pci\UhciDxe\UhciDxe\OUTPUT\.\ /showIncludes /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Gw /D DISABLE_NEW_DEPRECATED_INTERFACES /Id:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe  /Id:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\Bus\Pci\UhciDxe\UhciDxe\DEBUG  /Id:\edk2maintain\edk2\MdePkg  /Id:\edk2maintain\edk2\MdePkg\Include  /Id:\edk2maintain\edk2\MdePkg\Include\Ia32  /Id:\edk2maintain\edk2\MdeModulePkg  /Id:\edk2maintain\edk2\MdeModulePkg\Include d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\ComponentName.c d:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\Bus\Pci\UhciDxe\UhciDxe\DEBUG\AutoGen.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciReg.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciQueue.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\Uhci.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciDebug.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UsbHcMem.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciSched.c
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.exe"' : return code '0x2'
Stop.


build.py...
 : error 7000: Failed to execute command
        C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\nmake.exe /nologo tbuild [d:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand]


build.py...
 : error F002: Failed to build module
        d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.inf [IA32, VS2017, DEBUG]

- Failed -



Thanks,
Bob

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ard Biesheuvel
Sent: Saturday, March 7, 2020 10:22 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com>
Cc: Feng, Bob C <bob.c.feng@intel.com>
Subject: Re: [edk2-devel] [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

On Fri, 6 Mar 2020 at 20:22, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 03/06/20 17:40, Ard Biesheuvel wrote:
> > On Fri, 6 Mar 2020 at 17:14, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> 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/
> >>
> >
> > Are you sure? The const object has static linkage.
>
> Yes, I'm sure.
>

:-)

I should have known better than to doubt you on a matter like this ...

Thanks for the explanation.

Re: [edk2-devel] [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Laszlo Ersek]

Hi Bob,

On 03/08/20 02:22, Feng, Bob C wrote:
> Hi Ard,
> 
> With this patch, Ovmf still build failed on windows with VS2017. The error message like below:
> 
> Building ... d:\edk2maintain\edk2\MdeModulePkg\Universal\Metronome\Metronome.inf [IA32]
> d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.c(199): error C2220: warning treated as error - no 'object' file generated
> d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.c(199): warning C4244: '=': conversion from 'UINT64' to 'UINTN', possible loss of data
>         "C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.exe" /Fod:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\Bus\Pci\UhciDxe\UhciDxe\OUTPUT\.\ /showIncludes /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Gw /D DISABLE_NEW_DEPRECATED_INTERFACES /Id:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe  /Id:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\Bus\Pci\UhciDxe\UhciDxe\DEBUG  /Id:\edk2maintain\edk2\MdePkg  /Id:\edk2maintain\edk2\MdePkg\Include  /Id:\edk2maintain\edk2\MdePkg\Include\Ia32  /Id:\edk2maintain\edk2\MdeModulePkg  /Id:\edk2maintain\edk2\MdeModulePkg\Include d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\ComponentName.c d:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\Bus\Pci\UhciDxe\UhciDxe\DEBUG\AutoGen.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciReg.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciQueue.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\Uhci.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciDebug.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UsbHcMem.c d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciSched.c
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.exe"' : return code '0x2'
> Stop.
> 
> 
> build.py...
>  : error 7000: Failed to execute command
>         C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\nmake.exe /nologo tbuild [d:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand]
> 
> 
> build.py...
>  : error F002: Failed to build module
>         d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.inf [IA32, VS2017, DEBUG]
> 
> - Failed -

Can you please hack up a patch for OvmfPkg to suppress all of these
problems, post it as an RFC, and then we can clean it up as necessary?

We don't have access to VS2017. (I'm not sure if it's available free of
charge for download, but even if it is, last time I set up a VS
environment in a Windows guest, it took me a day or so.) Without access
to VS2017, it's very slow to solve such problems from our end, but you
could at least collect all the issues, because you can re-run the build
immediately after dealing with the most recently seen build error.

Hopefully this will get better once
<https://bugzilla.tianocore.org/show_bug.cgi?id=2570> is solved (i.e.
when OVMF is included in CI builds).

Thanks
Laszlo

Re: [edk2-devel] [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Bob Feng]

Laszlo,

I found this is the last one issue.  I created a patch https://edk2.groups.io/g/devel/message/55708. Please review.

Thanks,
Bob

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Sunday, March 8, 2020 5:00 PM
To: Feng, Bob C <bob.c.feng@intel.com>; devel@edk2.groups.io; ard.biesheuvel@linaro.org
Subject: Re: [edk2-devel] [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

Hi Bob,

On 03/08/20 02:22, Feng, Bob C wrote:
> Hi Ard,
> 
> With this patch, Ovmf still build failed on windows with VS2017. The error message like below:
> 
> Building ... 
> d:\edk2maintain\edk2\MdeModulePkg\Universal\Metronome\Metronome.inf 
> [IA32]
> d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitr
> dDynamicShellCommand.c(199): error C2220: warning treated as error - 
> no 'object' file generated
> d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitrdDynamicShellCommand.c(199): warning C4244: '=': conversion from 'UINT64' to 'UINTN', possible loss of data
>         "C:\Program Files (x86)\Microsoft Visual 
> Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.ex
> e" 
> /Fod:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\
> Bus\Pci\UhciDxe\UhciDxe\OUTPUT\.\ /showIncludes /nologo /arch:IA32 /c 
> /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- 
> /GF /Gy /Z7 /Gw /D DISABLE_NEW_DEPRECATED_INTERFACES 
> /Id:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe  
> /Id:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\B
> us\Pci\UhciDxe\UhciDxe\DEBUG  /Id:\edk2maintain\edk2\MdePkg  
> /Id:\edk2maintain\edk2\MdePkg\Include  
> /Id:\edk2maintain\edk2\MdePkg\Include\Ia32  
> /Id:\edk2maintain\edk2\MdeModulePkg  
> /Id:\edk2maintain\edk2\MdeModulePkg\Include 
> d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\ComponentName.c 
> d:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\MdeModulePkg\Bus
> \Pci\UhciDxe\UhciDxe\DEBUG\AutoGen.c 
> d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciReg.c 
> d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciQueue.c 
> d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\Uhci.c 
> d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciDebug.c 
> d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UsbHcMem.c 
> d:\edk2maintain\edk2\MdeModulePkg\Bus\Pci\UhciDxe\UhciSched.c
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\cl.exe"' : return code '0x2'
> Stop.
> 
> 
> build.py...
>  : error 7000: Failed to execute command
>         C:\Program Files (x86)\Microsoft Visual 
> Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\bin\Hostx86\x86\nmake
> .exe /nologo tbuild 
> [d:\edk2maintain\edk2\Build\OvmfIa32\DEBUG_VS2017\IA32\OvmfPkg\LinuxIn
> itrdDynamicShellCommand\LinuxInitrdDynamicShellCommand]
> 
> 
> build.py...
>  : error F002: Failed to build module
>         
> d:\edk2maintain\edk2\OvmfPkg\LinuxInitrdDynamicShellCommand\LinuxInitr
> dDynamicShellCommand.inf [IA32, VS2017, DEBUG]
> 
> - Failed -

Can you please hack up a patch for OvmfPkg to suppress all of these problems, post it as an RFC, and then we can clean it up as necessary?

We don't have access to VS2017. (I'm not sure if it's available free of charge for download, but even if it is, last time I set up a VS environment in a Windows guest, it took me a day or so.) Without access to VS2017, it's very slow to solve such problems from our end, but you could at least collect all the issues, because you can re-run the build immediately after dealing with the most recently seen build error.

Hopefully this will get better once
<https://bugzilla.tianocore.org/show_bug.cgi?id=2570> is solved (i.e.
when OVMF is included in CI builds).

Thanks
Laszlo

Re: [edk2-devel] [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

[Laszlo Ersek]

On 03/07/20 15:22, Ard Biesheuvel wrote:
> On Fri, 6 Mar 2020 at 20:22, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 03/06/20 17:40, Ard Biesheuvel wrote:
>>> On Fri, 6 Mar 2020 at 17:14, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>
>>>> 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/
>>>>
>>>
>>> Are you sure? The const object has static linkage.
>>
>> Yes, I'm sure.
>>
> 
> :-)
> 
> I should have known better than to doubt you on a matter like this ...

That's very kind :) but I do need to be kept on my toes!

> Thanks for the explanation.
> 

Thank you,
Laszlo