[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Changed malloc() to g_malloc() at places where
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] Changed malloc() to g_malloc() at places where return value was not being checked |
Date: |
Tue, 22 Mar 2016 14:57:04 +0000 |
On 22 March 2016 at 14:33, Md Haris Iqbal <address@hidden> wrote:
> Signed-off-by: Md Haris Iqbal <address@hidden>
Hi. I'm afraid you can't just change malloc() calls to
g_malloc() without also changing the corresponding free()
calls to g_free().
Also, at least the thunk.c code has had patches and
discussion on-list regarding its malloc() calls. It's
often worth searching the mailing list to check you won't
be repeating work that somebody else is already doing.
> ---
> bsd-user/elfload.c | 2 +-
> bsd-user/qemu.h | 2 +-
> linux-user/qemu.h | 2 +-
> thunk.c | 2 +-
> ui/shader.c | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 0a6092b..40bd1f2 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -1064,7 +1064,7 @@ static void load_symbols(struct elfhdr *hdr, int fd)
>
> found:
> /* Now know where the strtab and symtab are. Snarf them. */
> - s = malloc(sizeof(*s));
> + s = g_malloc(sizeof(*s));
> syms = malloc(symtab.sh_size);
> if (!syms) {
> free(s);
It's very odd to have only part of this data structure be
malloc and part g_malloc. We should convert all of it
or none of it.
> diff --git a/ui/shader.c b/ui/shader.c
> index 9264009..43c6f64 100644
> --- a/ui/shader.c
> +++ b/ui/shader.c
> @@ -83,7 +83,7 @@ GLuint qemu_gl_create_compile_shader(GLenum type, const
> GLchar *src)
> glGetShaderiv(shader, GL_COMPILE_STATUS, &status);
> if (!status) {
> glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &length);
> - errmsg = malloc(length);
> + errmsg = g_malloc(length);
> glGetShaderInfoLog(shader, length, &length, errmsg);
> fprintf(stderr, "%s: compile %s error\n%s\n", __func__,
> (type == GL_VERTEX_SHADER) ? "vertex" : "fragment",
Changes to the ui/shader.c code should be in a different
patch, since they're not related to the linux-user or
bsd-user code. (Splitting out bsd-user changes would
also be a good idea.) The idea here is that different
areas of the code have different maintainers, and so
review is by different people (and a problem in one part
of a patch doesn't then hold up an unrelated good change
in a different part).
thanks
-- PMM
Re: [Qemu-devel] [PATCH] Changed malloc() to g_malloc() at places where return value was not being checked, Markus Armbruster, 2016/03/22