diff mbox series

[v2,4/9] util: Return valid allocation for qemu_try_memalign() with zero size

Message ID 20220304112126.2261039-5-peter.maydell@linaro.org
State Superseded
Headers show
Series Cleanup of qemu_oom_check() and qemu_memalign() | expand

Commit Message

Peter Maydell March 4, 2022, 11:21 a.m. UTC 
Currently qemu_try_memalign()'s behaviour if asked to allocate
0 bytes is rather variable:
 * on Windows, we will assert
 * on POSIX platforms, we get the underlying behaviour of
   the posix_memalign() or equivalent function, which may be
   either "return a valid non-NULL pointer" or "return NULL"

Explictly check for 0 byte allocations, so we get consistent
behaviour across platforms.  We handle them by incrementing the size
so that we return a valid non-NULL pointer that can later be passed
to qemu_vfree().  This is permitted behaviour for the
posix_memalign() API and is the most usual way that underlying
malloc() etc implementations handle a zero-sized allocation request,
because it won't trip up calling code that assumes NULL means an
error.  (This includes our own qemu_memalign(), which will abort on
NULL.)

This change is a preparation for sharing the qemu_try_memalign() code
between Windows and POSIX.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 util/oslib-posix.c | 3 +++
 util/oslib-win32.c | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé March 4, 2022, 1:58 p.m. UTC | #1 
On 4/3/22 12:21, Peter Maydell wrote:
> Currently qemu_try_memalign()'s behaviour if asked to allocate
> 0 bytes is rather variable:
>   * on Windows, we will assert
>   * on POSIX platforms, we get the underlying behaviour of
>     the posix_memalign() or equivalent function, which may be
>     either "return a valid non-NULL pointer" or "return NULL"
> 
> Explictly check for 0 byte allocations, so we get consistent
> behaviour across platforms.  We handle them by incrementing the size
> so that we return a valid non-NULL pointer that can later be passed
> to qemu_vfree().  This is permitted behaviour for the
> posix_memalign() API and is the most usual way that underlying
> malloc() etc implementations handle a zero-sized allocation request,
> because it won't trip up calling code that assumes NULL means an
> error.  (This includes our own qemu_memalign(), which will abort on
> NULL.)
> 
> This change is a preparation for sharing the qemu_try_memalign() code
> between Windows and POSIX.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   util/oslib-posix.c | 3 +++
>   util/oslib-win32.c | 4 +++-
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 0278902ee79..f7e22f4ff9b 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -209,6 +209,9 @@ void *qemu_try_memalign(size_t alignment, size_t size)
>           g_assert(is_power_of_2(alignment));
>       }
>   
> +    if (size == 0) {
> +        size++;
> +    }
>   #if defined(CONFIG_POSIX_MEMALIGN)
>       int ret;
>       ret = posix_memalign(&ptr, alignment, size);
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 05857414695..8c28d70904d 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -48,12 +48,14 @@ void *qemu_try_memalign(size_t alignment, size_t size)
>   {
>       void *ptr;
>   
> -    g_assert(size != 0);

Better, X_try_Y() functions are not supposed to fail.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Richard Henderson March 4, 2022, 8:18 p.m. UTC | #2 
On 3/4/22 01:21, Peter Maydell wrote:
> Currently qemu_try_memalign()'s behaviour if asked to allocate
> 0 bytes is rather variable:
>   * on Windows, we will assert
>   * on POSIX platforms, we get the underlying behaviour of
>     the posix_memalign() or equivalent function, which may be
>     either "return a valid non-NULL pointer" or "return NULL"
> 
> Explictly check for 0 byte allocations, so we get consistent
> behaviour across platforms.  We handle them by incrementing the size
> so that we return a valid non-NULL pointer that can later be passed
> to qemu_vfree().  This is permitted behaviour for the
> posix_memalign() API and is the most usual way that underlying
> malloc() etc implementations handle a zero-sized allocation request,
> because it won't trip up calling code that assumes NULL means an
> error.  (This includes our own qemu_memalign(), which will abort on
> NULL.)
> 
> This change is a preparation for sharing the qemu_try_memalign() code
> between Windows and POSIX.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   util/oslib-posix.c | 3 +++
>   util/oslib-win32.c | 4 +++-
>   2 files changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
diff mbox series

Patch

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 0278902ee79..f7e22f4ff9b 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -209,6 +209,9 @@  void *qemu_try_memalign(size_t alignment, size_t size)
         g_assert(is_power_of_2(alignment));
     }
 
+    if (size == 0) {
+        size++;
+    }
 #if defined(CONFIG_POSIX_MEMALIGN)
     int ret;
     ret = posix_memalign(&ptr, alignment, size);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 05857414695..8c28d70904d 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -48,12 +48,14 @@  void *qemu_try_memalign(size_t alignment, size_t size)
 {
     void *ptr;
 
-    g_assert(size != 0);
     if (alignment < sizeof(void *)) {
         alignment = sizeof(void *);
     } else {
         g_assert(is_power_of_2(alignment));
     }
+    if (size == 0) {
+        size++;
+    }
     ptr = _aligned_malloc(size, alignment);
     trace_qemu_memalign(alignment, size, ptr);
     return ptr;