Re: [PATCH 1/2] util/osdep: Avoid madvise proto on modern Solaris

[Peter Maydell]

On Mon, 14 Mar 2022 at 16:12, Andrew Deason <adeason@sinenomine.net> wrote:
>
> On older Solaris releases, we didn't get a protype for madvise, and so
> util/osdep.c provides its own prototype. Some time between the public
> Solaris 11.4 release and Solaris 11.4.42 CBE, we started getting an
> madvise prototype that looks like this:
>
>     extern int madvise(void *, size_t, int);
>
> Which conflicts with the prototype in util/osdeps.c. Instead of always
> declaring this prototype, check if madvise() is already declared, so
> we don't need to declare it ourselves.
>
> Signed-off-by: Andrew Deason <adeason@sinenomine.net>
> ---
> I'm not sure if a test is needed for this at all; that is, how much qemu
> cares about earlier Solaris. The madvise prototype exists earlier in
> Solaris 11 (I'm not sure when it started appearing usefully), but in
> 11.4 and earlier it was compatible with the char* prototype.

>  #ifdef CONFIG_SOLARIS
>  #include <sys/statvfs.h>
> +#ifndef HAVE_MADVISE_PROTO
>  /* See MySQL bug #7156 (http://bugs.mysql.com/bug.php?id=7156) for
>     discussion about Solaris header problems */
>  extern int madvise(char *, size_t, int);
>  #endif
> +#endif

Rather than keeping this inside a CONFIG_SOLARIS and only doing
the meson.build test if targetos == sunos, I would prefer it if we
unconditionally determined two things in meson.build:
 (1) do we have madvise in the usual way? (this is what we would
     want CONFIG_MADVISE to be, and might even be what it actually is)
 (2) do we have madvise but only if we provide a prototype for it
     ourselves? (maybe CONFIG_MADVISE_NO_PROTO)

and then in osdep.h provide the prototype if CONFIG_MADVISE_NO_PROTO.
(osdep.h is where we provide "this is a fixup to the system headers"
portability workarounds, which this seems to be.)

This isn't the only .c file that directly calls madvise() :
softmmu/physmem.c does also. That looks like maybe a bug though:
perhaps it should be calling qemu_madvise()...

Side note: do you know why CONFIG_SOLARIS includes sys/statvfs.h ?
Is that unrelated to madvise() ?

thanks
-- PMM