On 2024/06/01 0: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.
Although this approach seems reasonable, so:
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
We should use plain madvise() if posix_madvise() is broken. In fact,
QEMU detects the availability of plain madvise() and use it instead of
posix_madvise() on my MacBook.
Perhaps it may be better to stop defining CONFIG_POSIX_MADVISE on Darwin
to ensure we never use the broken implementation.
