On 22/03/2016 16:52, haris iqbal wrote:
> On Tue, Mar 22, 2016 at 8:27 PM, Peter Maydell <peter.mayd...@linaro.org>
> wrote:
>> On 22 March 2016 at 14:33, Md Haris Iqbal <haris.p...@gmail.com> wrote:
>>> Signed-off-by: Md Haris Iqbal <haris.p...@gmail.com>
>>
>> Hi. I'm afraid you can't just change malloc() calls to
>> g_malloc() without also changing the corresponding free()
>> calls to g_free().
>
> Ok, I will do that.
>
>>
>> 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);
>
> I did that because the other one has a return value check. Which (as
> discussed with Paolo Bonzini) should be done in a seperate patch.
In this case you can convert syms to g_try_malloc.
Paolo
>>
>> 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).
>
> Ya, sure. I will split up the patches for different files and send
> them. I can cc the seperate maintainer also then.
>
>>
>> thanks
>> -- PMM
>
>
>
