[PATCH 1/1] OvmfPkg/VirtioSerialDxe: fix RELEASE build error

[PATCH 1/1] OvmfPkg/VirtioSerialDxe: fix RELEASE build error

[Gerd Hoffmann]

EventNames is used to pretty-print debug log messages.
Add #ifdef to only include it in debug builds.
Fixes a clang build failure.

Reported-by: Rebecca Cran <rebecca@bsdio.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/VirtioSerialDxe/VirtioSerial.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/OvmfPkg/VirtioSerialDxe/VirtioSerial.c b/OvmfPkg/VirtioSerialDxe/VirtioSerial.c
index bfb2b324eadf..5fb4ce31914b 100644
--- a/OvmfPkg/VirtioSerialDxe/VirtioSerial.c
+++ b/OvmfPkg/VirtioSerialDxe/VirtioSerial.c
@@ -25,6 +25,7 @@
 
 STATIC LIST_ENTRY  mVirtioSerialList;
 
+#if !defined (MDEPKG_NDEBUG)
 STATIC CONST CHAR8  *EventNames[] = {
   [VIRTIO_SERIAL_DEVICE_READY]  = "device-ready",
   [VIRTIO_SERIAL_DEVICE_ADD]    = "device-add",
@@ -35,6 +36,7 @@ STATIC CONST CHAR8  *EventNames[] = {
   [VIRTIO_SERIAL_PORT_OPEN]     = "port-open",
   [VIRTIO_SERIAL_PORT_NAME]     = "port-name",
 };
+#endif
 
 VOID
 EFIAPI
-- 
2.40.1

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioSerialDxe: fix RELEASE build error

[Ard Biesheuvel]

On Wed, 10 May 2023 at 09:34, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> EventNames is used to pretty-print debug log messages.
> Add #ifdef to only include it in debug builds.
> Fixes a clang build failure.
>
> Reported-by: Rebecca Cran <rebecca@bsdio.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Queued up as #4381 - thanks.


> ---
>  OvmfPkg/VirtioSerialDxe/VirtioSerial.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/OvmfPkg/VirtioSerialDxe/VirtioSerial.c b/OvmfPkg/VirtioSerialDxe/VirtioSerial.c
> index bfb2b324eadf..5fb4ce31914b 100644
> --- a/OvmfPkg/VirtioSerialDxe/VirtioSerial.c
> +++ b/OvmfPkg/VirtioSerialDxe/VirtioSerial.c
> @@ -25,6 +25,7 @@
>
>  STATIC LIST_ENTRY  mVirtioSerialList;
>
> +#if !defined (MDEPKG_NDEBUG)
>  STATIC CONST CHAR8  *EventNames[] = {
>    [VIRTIO_SERIAL_DEVICE_READY]  = "device-ready",
>    [VIRTIO_SERIAL_DEVICE_ADD]    = "device-add",
> @@ -35,6 +36,7 @@ STATIC CONST CHAR8  *EventNames[] = {
>    [VIRTIO_SERIAL_PORT_OPEN]     = "port-open",
>    [VIRTIO_SERIAL_PORT_NAME]     = "port-name",
>  };
> +#endif
>
>  VOID
>  EFIAPI
> --
> 2.40.1
>
>
>
> 
>
>

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioSerialDxe: fix RELEASE build error

[Ard Biesheuvel]

On Wed, 10 May 2023 at 09:39, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 10 May 2023 at 09:34, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > EventNames is used to pretty-print debug log messages.
> > Add #ifdef to only include it in debug builds.
> > Fixes a clang build failure.
> >
> > Reported-by: Rebecca Cran <rebecca@bsdio.com>
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Queued up as #4381 - thanks.
>

This appears to break the build

https://github.com/tianocore/edk2/pull/4381

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioSerialDxe: fix RELEASE build error

[Ard Biesheuvel]

On Wed, 10 May 2023 at 11:18, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 10 May 2023 at 09:39, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 10 May 2023 at 09:34, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > EventNames is used to pretty-print debug log messages.
> > > Add #ifdef to only include it in debug builds.
> > > Fixes a clang build failure.
> > >
> > > Reported-by: Rebecca Cran <rebecca@bsdio.com>
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Queued up as #4381 - thanks.
> >
>
> This appears to break the build
>
> https://github.com/tianocore/edk2/pull/4381

Actually, looking at the code, I think the Clang diagnostic is highly
dubious here. The non-DEBUG code takes the ARRAY_SIZE() of EventNames,
and so the #ifdef will result in a compiler error.

I suppose we can work around this, but we might also just add
-Wunneeded-internal-declaration to the clang cflags for RELEASE

Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtioSerialDxe: fix RELEASE build error

[Ard Biesheuvel]

On Wed, 10 May 2023 at 15:54, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 10 May 2023 at 11:18, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 10 May 2023 at 09:39, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 10 May 2023 at 09:34, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > >
> > > > EventNames is used to pretty-print debug log messages.
> > > > Add #ifdef to only include it in debug builds.
> > > > Fixes a clang build failure.
> > > >
> > > > Reported-by: Rebecca Cran <rebecca@bsdio.com>
> > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > >
> > > Queued up as #4381 - thanks.
> > >
> >
> > This appears to break the build
> >
> > https://github.com/tianocore/edk2/pull/4381
>
> Actually, looking at the code, I think the Clang diagnostic is highly
> dubious here. The non-DEBUG code takes the ARRAY_SIZE() of EventNames,
> and so the #ifdef will result in a compiler error.
>
> I suppose we can work around this, but we might also just add
> -Wunneeded-internal-declaration to the clang cflags for RELEASE

I've sent a BaseTools patch for this - I suggest we merge that and
drop this one.