On 5/31/24 17:46, Philippe Mathieu-Daudé wrote: > On 31/5/24 17:10, Michal Privoznik wrote: >> The unspoken premise of qemu_madvise() is that errno is set on >> error. And it is mostly the case except for posix_madvise() which >> is documented to return either zero (on success) or a positive >> error number. This means, we must set errno ourselves. And while >> at it, make the function return a negative value on error, just >> like other error paths do. >> >> Signed-off-by: Michal Privoznik <mpriv...@redhat.com> >> --- >> util/osdep.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/util/osdep.c b/util/osdep.c >> index e996c4744a..1345238a5c 100644 >> --- a/util/osdep.c >> +++ b/util/osdep.c >> @@ -57,7 +57,19 @@ int qemu_madvise(void *addr, size_t len, int advice) >> #if defined(CONFIG_MADVISE) >> return madvise(addr, len, advice); >> #elif defined(CONFIG_POSIX_MADVISE) >> - return posix_madvise(addr, len, advice); >> + /* >> + * On Darwin posix_madvise() has the same return semantics as >> + * plain madvise, i.e. errno is set and -1 is returned. Otherwise, >> + * a positive error number is returned. >> + */ > > Alternative is to guard with #ifdef CONFIG_DARWIN ... #else ... #endif > which might be clearer.
That's how I had it written (locally) initially, but then thought: well, what if there's another OS that behaves the same? This way, we don't have to care and just do the right thing. > > Although this approach seems reasonable, so: > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> Thanks! Michal
