[PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[Philippe Mathieu-Daudé]

When liblzfe (Apple LZFSE compression library) is present
(for example installed via 'brew') on Darwin, QEMU build
fails as:

  Has header "lzfse.h" : YES
  Library lzfse found: YES

    Dependencies
      lzo support                  : NO
      snappy support               : NO
      bzip2 support                : YES
      lzfse support                : YES
      zstd support                 : YES 1.5.2

    User defined options
      dmg                          : enabled
      lzfse                        : enabled

  [221/903] Compiling C object libblock.fa.p/block_dmg-lzfse.c.o
  FAILED: libblock.fa.p/block_dmg-lzfse.c.o
  /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:56:43: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
  LZFSE_API size_t lzfse_encode_scratch_size();
                                            ^
                                             void
  /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:94:43: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
  LZFSE_API size_t lzfse_decode_scratch_size();
                                            ^
                                             void
  2 errors generated.
  ninja: build stopped: subcommand failed.

This issue has been reported in the lzfse project in 2016:
https://github.com/lzfse/lzfse/issues/3#issuecomment-226574719

Since the project seems unmaintained, simply ignore the
strict-prototypes warning check for the <lzfse.h> header,
similarly to how we deal with the GtkItemFactoryCallback
prototype from <gtk/gtkitemfactory.h>, indirectly included
by <gtk/gtk.h>.

Cc: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 block/dmg-lzfse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
index 6798cf4fbf..0abc970bf6 100644
--- a/block/dmg-lzfse.c
+++ b/block/dmg-lzfse.c
@@ -23,7 +23,12 @@
  */
 #include "qemu/osdep.h"
 #include "dmg.h"
+
+/* Work around an -Wstrict-prototypes warning in LZFSE headers */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstrict-prototypes"
 #include <lzfse.h>
+#pragma GCC diagnostic pop
 
 static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in,
                                    char *next_out, unsigned int avail_out)
-- 
2.38.1

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[Stefan Weil via]

Am 27.03.23 um 17:13 schrieb Philippe Mathieu-Daudé:

> When liblzfe (Apple LZFSE compression library) is present
> (for example installed via 'brew') on Darwin, QEMU build
> fails as:
>
>    Has header "lzfse.h" : YES
>    Library lzfse found: YES
>
>      Dependencies
>        lzo support                  : NO
>        snappy support               : NO
>        bzip2 support                : YES
>        lzfse support                : YES
>        zstd support                 : YES 1.5.2
>
>      User defined options
>        dmg                          : enabled
>        lzfse                        : enabled
>
>    [221/903] Compiling C object libblock.fa.p/block_dmg-lzfse.c.o
>    FAILED: libblock.fa.p/block_dmg-lzfse.c.o
>    /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:56:43: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
>    LZFSE_API size_t lzfse_encode_scratch_size();
>                                              ^
>                                               void
>    /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:94:43: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
>    LZFSE_API size_t lzfse_decode_scratch_size();
>                                              ^
>                                               void
>    2 errors generated.
>    ninja: build stopped: subcommand failed.
>
> This issue has been reported in the lzfse project in 2016:
> https://github.com/lzfse/lzfse/issues/3#issuecomment-226574719
>
> Since the project seems unmaintained, simply ignore the
> strict-prototypes warning check for the <lzfse.h> header,
> similarly to how we deal with the GtkItemFactoryCallback
> prototype from <gtk/gtkitemfactory.h>, indirectly included
> by <gtk/gtk.h>.
>
> Cc: Julio Faracco <jcfaracco@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   block/dmg-lzfse.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
> index 6798cf4fbf..0abc970bf6 100644
> --- a/block/dmg-lzfse.c
> +++ b/block/dmg-lzfse.c
> @@ -23,7 +23,12 @@
>    */
>   #include "qemu/osdep.h"
>   #include "dmg.h"
> +
> +/* Work around an -Wstrict-prototypes warning in LZFSE headers */


"Work around a -Wstrict-prototypes" ("a" instead of "an")?


> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstrict-prototypes"
>   #include <lzfse.h>
> +#pragma GCC diagnostic pop
>   
>   static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in,
>                                      char *next_out, unsigned int avail_out)


The warning can also be suppressed if the build uses `-isystem 
/opt/homebrew/include` instead of `-I/opt/homebrew/include` as I just 
have tested.

If we can find a solution how to implement that I thing it would look 
nicer. Technically the patch looks good.

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Thanks,

Stefan

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[Philippe Mathieu-Daudé]

+Marc-André & Paolo

On 27/3/23 19:08, Stefan Weil wrote:
> Am 27.03.23 um 17:13 schrieb Philippe Mathieu-Daudé:
> 
>> When liblzfe (Apple LZFSE compression library) is present
>> (for example installed via 'brew') on Darwin, QEMU build
>> fails as:
>>
>>    Has header "lzfse.h" : YES
>>    Library lzfse found: YES
>>
>>      Dependencies
>>        lzo support                  : NO
>>        snappy support               : NO
>>        bzip2 support                : YES
>>        lzfse support                : YES
>>        zstd support                 : YES 1.5.2
>>
>>      User defined options
>>        dmg                          : enabled
>>        lzfse                        : enabled
>>
>>    [221/903] Compiling C object libblock.fa.p/block_dmg-lzfse.c.o
>>    FAILED: libblock.fa.p/block_dmg-lzfse.c.o
>>    /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:56:43: error: this 
>> function declaration is not a prototype [-Werror,-Wstrict-prototypes]
>>    LZFSE_API size_t lzfse_encode_scratch_size();
>>                                              ^
>>                                               void
>>    /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:94:43: error: this 
>> function declaration is not a prototype [-Werror,-Wstrict-prototypes]
>>    LZFSE_API size_t lzfse_decode_scratch_size();
>>                                              ^
>>                                               void
>>    2 errors generated.
>>    ninja: build stopped: subcommand failed.
>>
>> This issue has been reported in the lzfse project in 2016:
>> https://github.com/lzfse/lzfse/issues/3#issuecomment-226574719
>>
>> Since the project seems unmaintained, simply ignore the
>> strict-prototypes warning check for the <lzfse.h> header,
>> similarly to how we deal with the GtkItemFactoryCallback
>> prototype from <gtk/gtkitemfactory.h>, indirectly included
>> by <gtk/gtk.h>.
>>
>> Cc: Julio Faracco <jcfaracco@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   block/dmg-lzfse.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
>> index 6798cf4fbf..0abc970bf6 100644
>> --- a/block/dmg-lzfse.c
>> +++ b/block/dmg-lzfse.c
>> @@ -23,7 +23,12 @@
>>    */
>>   #include "qemu/osdep.h"
>>   #include "dmg.h"
>> +
>> +/* Work around an -Wstrict-prototypes warning in LZFSE headers */
> 
> 
> "Work around a -Wstrict-prototypes" ("a" instead of "an")?
> 
> 
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wstrict-prototypes"
>>   #include <lzfse.h>
>> +#pragma GCC diagnostic pop
>>   static int dmg_uncompress_lzfse_do(char *next_in, unsigned int 
>> avail_in,
>>                                      char *next_out, unsigned int 
>> avail_out)
> 
> 
> The warning can also be suppressed if the build uses `-isystem 
> /opt/homebrew/include` instead of `-I/opt/homebrew/include` as I just 
> have tested.

IIUC by design meson only allows including *relative* directories,
and manage the system ones:
https://mesonbuild.com/Include-directories.html

> If we can find a solution how to implement that I thing it would look 
> nicer. Technically the patch looks good.
> 
> Reviewed-by: Stefan Weil <sw@weilnetz.de>

Thanks!

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

Il lun 27 mar 2023, 20:58 Philippe Mathieu-Daudé <philmd@linaro.org> ha
scritto:

> > The warning can also be suppressed if the build uses `-isystem
> > /opt/homebrew/include` instead of `-I/opt/homebrew/include` as I just
> > have tested.
>

Is that option added by QEMU's configure or meson.build? Or is it added by
homebrew? The fact that /opt/homebrew/include it isn't considered a system
seems to be a homebrew decision.

IIUC by design meson only allows including *relative* directories,
> and manage the system ones:
> https://mesonbuild.com/Include-directories.html


That's for includes that are part of QEMU.

Meson has as_system for dependency objects (
https://mesonbuild.com/Reference-manual_returned_dep.html) but lzfse
doesn't have a .pc file, its detection has to be done by hand.

Paolo


> > If we can find a solution how to implement that I thing it would look
> > nicer. Technically the patch looks good.
> >
> > Reviewed-by: Stefan Weil <sw@weilnetz.de>
>
> Thanks!
>
>

[-- Attachment #2: Type: text/html, Size: 2166 bytes --]

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[Stefan Weil via]

[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]

Am 27.03.23 um 23:09 schrieb Paolo Bonzini:

> Il lun 27 mar 2023, 20:58 Philippe Mathieu-Daudé <philmd@linaro.org> 
> ha scritto:
>
>     > The warning can also be suppressed if the build uses `-isystem
>     > /opt/homebrew/include` instead of `-I/opt/homebrew/include` as I
>     just
>     > have tested.
>
> Is that option added by QEMU's configure or meson.build? Or is it 
> added by homebrew? The fact that /opt/homebrew/include it isn't 
> considered a system seems to be a homebrew decision.
>
>     IIUC by design meson only allows including *relative* directories,
>     and manage the system ones:
>     https://mesonbuild.com/Include-directories.html
>
> That's for includes that are part of QEMU.
>
> Meson has as_system for dependency objects 
> (https://mesonbuild.com/Reference-manual_returned_dep.html) but lzfse 
> doesn't have a .pc file, its detection has to be done by hand.
>
> Paolo
>
>     > If we can find a solution how to implement that I thing it would
>     look
>     > nicer. Technically the patch looks good.
>     >
>     > Reviewed-by: Stefan Weil <sw@weilnetz.de>
>
>     Thanks!
>

Typically I configure the build on macOS with `./configure 
--extra-cflags=-I/opt/homebrew/include 
--extra-ldflags=-L/opt/homebrew/lib --disable-werror`. With that 
configuration I get the two warnings for lzfse.h.

When I use `./configure '--extra-cflags=-isystem /opt/homebrew/include' 
--extra-ldflags=-L/opt/homebrew/lib --disable-werror` instead, I get no 
compiler warnings (and `--disable-werror` could be ommitted).

So at least for macOS with Homebrew in /opt/homebrew (M1 / M2 Macs) the 
patch is not needed when the right configure options (`--extra-cflags`) 
were used.

Stefan


[-- Attachment #2: Type: text/html, Size: 3671 bytes --]

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[Philippe Mathieu-Daudé]

On 28/3/23 08:29, Stefan Weil wrote:
> Am 27.03.23 um 23:09 schrieb Paolo Bonzini:
> 
>> Il lun 27 mar 2023, 20:58 Philippe Mathieu-Daudé <philmd@linaro.org> 
>> ha scritto:
>>
>>     > The warning can also be suppressed if the build uses `-isystem
>>     > /opt/homebrew/include` instead of `-I/opt/homebrew/include` as I
>>     just
>>     > have tested.
>>
>> Is that option added by QEMU's configure or meson.build? Or is it 
>> added by homebrew? The fact that /opt/homebrew/include it isn't 
>> considered a system seems to be a homebrew decision.
>>
>>     IIUC by design meson only allows including *relative* directories,
>>     and manage the system ones:
>>     https://mesonbuild.com/Include-directories.html
>>
>> That's for includes that are part of QEMU.
>>
>> Meson has as_system for dependency objects 
>> (https://mesonbuild.com/Reference-manual_returned_dep.html) but lzfse 
>> doesn't have a .pc file, its detection has to be done by hand.
>>
>> Paolo
>>
>>     > If we can find a solution how to implement that I thing it would
>>     look
>>     > nicer. Technically the patch looks good.
>>     >
>>     > Reviewed-by: Stefan Weil <sw@weilnetz.de>
>>
>>     Thanks!
>>
> 
> Typically I configure the build on macOS with `./configure 
> --extra-cflags=-I/opt/homebrew/include 
> --extra-ldflags=-L/opt/homebrew/lib --disable-werror`. With that 
> configuration I get the two warnings for lzfse.h.
> 
> When I use `./configure '--extra-cflags=-isystem /opt/homebrew/include' 
> --extra-ldflags=-L/opt/homebrew/lib --disable-werror` instead, I get no 
> compiler warnings (and `--disable-werror` could be ommitted).
> 
> So at least for macOS with Homebrew in /opt/homebrew (M1 / M2 Macs) the 
> patch is not needed when the right configure options (`--extra-cflags`) 
> were used.

What I learned:

- If lzfse were well packaged (as noted Paolo), we could use
   dependency(..., include_type: 'system')
   https://github.com/mesonbuild/meson/issues/963#issuecomment-1277851401

- I agree with Eli Schwartz, a Meson maintainer:

   > More bugs caused by and only affecting people who misuse and
   > abuse -isystem as "not system, but as a side effect please
   > don't emit warnings for this messy dependency"...

   https://github.com/mesonbuild/meson/issues/8755#issuecomment-836913759

I wasted few hours on this, and am now giving up in favor of this simple
patch.

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from <lzfse.h>

[Philippe Mathieu-Daudé]

On 27/3/23 17:13, Philippe Mathieu-Daudé wrote:
> When liblzfe (Apple LZFSE compression library) is present
> (for example installed via 'brew') on Darwin, QEMU build
> fails as:


>    [221/903] Compiling C object libblock.fa.p/block_dmg-lzfse.c.o
>    FAILED: libblock.fa.p/block_dmg-lzfse.c.o
>    /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:56:43: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
>    LZFSE_API size_t lzfse_encode_scratch_size();
>                                              ^
>                                               void
>    /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:94:43: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
>    LZFSE_API size_t lzfse_decode_scratch_size();
>                                              ^
>                                               void
>    2 errors generated.
>    ninja: build stopped: subcommand failed.
> 
> This issue has been reported in the lzfse project in 2016:
> https://github.com/lzfse/lzfse/issues/3#issuecomment-226574719
> 
> Since the project seems unmaintained, simply ignore the
> strict-prototypes warning check for the <lzfse.h> header,
> similarly to how we deal with the GtkItemFactoryCallback
> prototype from <gtk/gtkitemfactory.h>, indirectly included
> by <gtk/gtk.h>.
> 
> Cc: Julio Faracco <jcfaracco@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   block/dmg-lzfse.c | 5 +++++
>   1 file changed, 5 insertions(+)

Patch queued for 8.0-rc3.