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

[Andrew Deason]

On Mon, 14 Mar 2022 16:36:00 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Mon, 14 Mar 2022 at 16:12, Andrew Deason <adeason@sinenomine.net> wrote:
> >  #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)

CONFIG_MADVISE is set if we can cc.links() something that calls
madvise(). If we're missing the prototype, that will fail with -Werror,
but I expect succeeds otherwise. If cc.links() just uses the cflags for
the build, then it seems like it might succeed or fail depending on
--enable-werror. I see some other tests give -Werror as an explicit
extra argument (HAVE_BROKEN_SIZE_MAX, and something for fuzzing); should
this be doing the same to make sure it fails for a missing prototype?

Also just to mention, if we don't care about older Solaris, the
prototype can just be unconditionally removed. It's pretty annoying to
even try to build qemu from git on Solaris 11.4 and earlier, because
many of the build requirements need to be installed/compiled manually
(notably python 3.6+, but iirc also ninja, meson, etc). So I haven't
really tried; there may be many other build issues there.

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

I think so, it was added in commit 605686cd7ac, which predates madvise()
in that file. It does look like it could be removed from a quick glance.

Would you want me to add a commit to remove it? (Assuming it still
compiles okay.) Or just do that in the same commit as changing the
madvise prototype logic?

-- 
Andrew Deason
adeason@sinenomine.net