Re: [PATCH v3 1/4] osdep: Make qemu_madvise() to set errno in all cases

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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 <mprivozn@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é <philmd@linaro.org>

> +    int rc = posix_madvise(addr, len, advice);
> +    if (rc) {
> +        if (rc > 0) {
> +            errno = rc;
> +        }
> +        return -1;
> +    }
> +    return 0;
>   #else
>       errno = EINVAL;
>       return -1;