[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] util/compatfd.c: Replaced a malloc call with g_malloc.
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 2/2] util/compatfd.c: Replaced a malloc call with g_malloc. |
Date: |
Mon, 15 Mar 2021 12:13:53 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 |
Hi Mahmoud,
On 3/15/21 11:58 AM, Mahmoud Mandour wrote:
> Replaced a call to malloc() and its respective call to free()
> with g_malloc() and g_free().
>
> g_malloc() is preferred more than g_try_* functions, which
> return NULL on error, when the size of the requested
> allocation is small. This is because allocating few
> bytes should not be a problem in a healthy system.
> Otherwise, the system is already in a critical state.
>
> Subsequently, removed NULL-checking after g_malloc().
>
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> ---
> util/compatfd.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/util/compatfd.c b/util/compatfd.c
> index 174f394533..a8ec525c6c 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -72,14 +72,10 @@ static int qemu_signalfd_compat(const sigset_t *mask)
> QemuThread thread;
> int fds[2];
>
> - info = malloc(sizeof(*info));
> - if (info == NULL) {
> - errno = ENOMEM;
> - return -1;
> - }
> + info = g_malloc(sizeof(*info));
Watch out...
https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html
If any call to allocate memory using functions g_new(), g_new0(),
g_renew(), g_malloc(), g_malloc0(), g_malloc0_n(), g_realloc(),
and g_realloc_n() fails, the application is terminated.
So with your change instead of handling ENOMEM the QEMU process is
simply killed.
Don't you want to use g_try_new(struct sigfd_compat_info, 1) here
instead?
>
> if (pipe(fds) == -1) {
> - free(info);
> + g_free(info);
> return -1;
> }
>
>