* [PATCH] macfb: fix a memory leak (CID 1465231)
@ 2021-11-05 16:52 Laurent Vivier
2021-11-05 17:01 ` Peter Maydell
2021-11-08 8:06 ` Mark Cave-Ayland
0 siblings, 2 replies; 5+ messages in thread
From: Laurent Vivier @ 2021-11-05 16:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Mark Cave-Ayland, Laurent Vivier
Rewrite the function using g_string_append_printf() rather than
g_strdup_printf()/g_strconcat().
Fixes: df8abbbadf74 ("macfb: add common monitor modes supported by the MacOS toolbox ROM")
Cc: mark.cave-ayland@ilande.co.uk
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
hw/display/macfb.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/display/macfb.c b/hw/display/macfb.c
index 4b352eb89c3f..277d3e663331 100644
--- a/hw/display/macfb.c
+++ b/hw/display/macfb.c
@@ -440,21 +440,18 @@ static MacFbMode *macfb_find_mode(MacfbDisplayType display_type,
static gchar *macfb_mode_list(void)
{
- gchar *list = NULL;
- gchar *mode;
+ GString *list = g_string_new("");
MacFbMode *macfb_mode;
int i;
for (i = 0; i < ARRAY_SIZE(macfb_mode_table); i++) {
macfb_mode = &macfb_mode_table[i];
- mode = g_strdup_printf(" %dx%dx%d\n", macfb_mode->width,
+ g_string_append_printf(list, " %dx%dx%d\n", macfb_mode->width,
macfb_mode->height, macfb_mode->depth);
- list = g_strconcat(mode, list, NULL);
- g_free(mode);
}
- return list;
+ return g_string_free(list, FALSE);
}
@@ -643,7 +640,7 @@ static bool macfb_common_realize(DeviceState *dev, MacfbState *s, Error **errp)
gchar *list;
error_setg(errp, "unknown display mode: width %d, height %d, depth %d",
s->width, s->height, s->depth);
- list = macfb_mode_list();
+ list = macfb_mode_list();
error_append_hint(errp, "Available modes:\n%s", list);
g_free(list);
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] macfb: fix a memory leak (CID 1465231)
2021-11-05 16:52 [PATCH] macfb: fix a memory leak (CID 1465231) Laurent Vivier
@ 2021-11-05 17:01 ` Peter Maydell
2021-11-05 18:32 ` Laurent Vivier
2021-11-08 8:06 ` Mark Cave-Ayland
1 sibling, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2021-11-05 17:01 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Mark Cave-Ayland, qemu-devel
On Fri, 5 Nov 2021 at 16:52, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Rewrite the function using g_string_append_printf() rather than
> g_strdup_printf()/g_strconcat().
>
> Fixes: df8abbbadf74 ("macfb: add common monitor modes supported by the MacOS toolbox ROM")
> Cc: mark.cave-ayland@ilande.co.uk
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> hw/display/macfb.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 4b352eb89c3f..277d3e663331 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -440,21 +440,18 @@ static MacFbMode *macfb_find_mode(MacfbDisplayType display_type,
>
> static gchar *macfb_mode_list(void)
> {
> - gchar *list = NULL;
> - gchar *mode;
> + GString *list = g_string_new("");
> MacFbMode *macfb_mode;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(macfb_mode_table); i++) {
> macfb_mode = &macfb_mode_table[i];
>
> - mode = g_strdup_printf(" %dx%dx%d\n", macfb_mode->width,
> + g_string_append_printf(list, " %dx%dx%d\n", macfb_mode->width,
> macfb_mode->height, macfb_mode->depth);
> - list = g_strconcat(mode, list, NULL);
> - g_free(mode);
> }
>
> - return list;
> + return g_string_free(list, FALSE);
This reverses the order compared to the old code (which prepends
'mode' to the 'list' string it is building up). Does that matter ?
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] macfb: fix a memory leak (CID 1465231)
2021-11-05 17:01 ` Peter Maydell
@ 2021-11-05 18:32 ` Laurent Vivier
2021-11-05 18:43 ` Peter Maydell
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2021-11-05 18:32 UTC (permalink / raw)
To: Peter Maydell; +Cc: Mark Cave-Ayland, qemu-devel
Le 05/11/2021 à 18:01, Peter Maydell a écrit :
> On Fri, 5 Nov 2021 at 16:52, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Rewrite the function using g_string_append_printf() rather than
>> g_strdup_printf()/g_strconcat().
>>
>> Fixes: df8abbbadf74 ("macfb: add common monitor modes supported by the MacOS toolbox ROM")
>> Cc: mark.cave-ayland@ilande.co.uk
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>> hw/display/macfb.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
>> index 4b352eb89c3f..277d3e663331 100644
>> --- a/hw/display/macfb.c
>> +++ b/hw/display/macfb.c
>> @@ -440,21 +440,18 @@ static MacFbMode *macfb_find_mode(MacfbDisplayType display_type,
>>
>> static gchar *macfb_mode_list(void)
>> {
>> - gchar *list = NULL;
>> - gchar *mode;
>> + GString *list = g_string_new("");
>> MacFbMode *macfb_mode;
>> int i;
>>
>> for (i = 0; i < ARRAY_SIZE(macfb_mode_table); i++) {
>> macfb_mode = &macfb_mode_table[i];
>>
>> - mode = g_strdup_printf(" %dx%dx%d\n", macfb_mode->width,
>> + g_string_append_printf(list, " %dx%dx%d\n", macfb_mode->width,
>> macfb_mode->height, macfb_mode->depth);
>> - list = g_strconcat(mode, list, NULL);
>> - g_free(mode);
>> }
>>
>> - return list;
>> + return g_string_free(list, FALSE);
>
> This reverses the order compared to the old code (which prepends
> 'mode' to the 'list' string it is building up). Does that matter ?
>
Not at all. Perhaps it's even better like that as we have lower resolutions first.
It was done like that to be able to pass list set to NULL (first parameter must not be NULL).
Thanks,
Laurent
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] macfb: fix a memory leak (CID 1465231)
2021-11-05 18:32 ` Laurent Vivier
@ 2021-11-05 18:43 ` Peter Maydell
0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2021-11-05 18:43 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Mark Cave-Ayland, qemu-devel
On Fri, 5 Nov 2021 at 18:32, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 05/11/2021 à 18:01, Peter Maydell a écrit :
> > On Fri, 5 Nov 2021 at 16:52, Laurent Vivier <laurent@vivier.eu> wrote:
> >>
> >> Rewrite the function using g_string_append_printf() rather than
> >> g_strdup_printf()/g_strconcat().
> >>
> >> Fixes: df8abbbadf74 ("macfb: add common monitor modes supported by the MacOS toolbox ROM")
> >> Cc: mark.cave-ayland@ilande.co.uk
> >> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> >> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> >> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> >> ---
> >> hw/display/macfb.c | 11 ++++-------
> >> 1 file changed, 4 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> >> index 4b352eb89c3f..277d3e663331 100644
> >> --- a/hw/display/macfb.c
> >> +++ b/hw/display/macfb.c
> >> @@ -440,21 +440,18 @@ static MacFbMode *macfb_find_mode(MacfbDisplayType display_type,
> >>
> >> static gchar *macfb_mode_list(void)
> >> {
> >> - gchar *list = NULL;
> >> - gchar *mode;
> >> + GString *list = g_string_new("");
> >> MacFbMode *macfb_mode;
> >> int i;
> >>
> >> for (i = 0; i < ARRAY_SIZE(macfb_mode_table); i++) {
> >> macfb_mode = &macfb_mode_table[i];
> >>
> >> - mode = g_strdup_printf(" %dx%dx%d\n", macfb_mode->width,
> >> + g_string_append_printf(list, " %dx%dx%d\n", macfb_mode->width,
> >> macfb_mode->height, macfb_mode->depth);
> >> - list = g_strconcat(mode, list, NULL);
> >> - g_free(mode);
> >> }
> >>
> >> - return list;
> >> + return g_string_free(list, FALSE);
> >
> > This reverses the order compared to the old code (which prepends
> > 'mode' to the 'list' string it is building up). Does that matter ?
> >
>
> Not at all. Perhaps it's even better like that as we have lower resolutions first.
In that case,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] macfb: fix a memory leak (CID 1465231)
2021-11-05 16:52 [PATCH] macfb: fix a memory leak (CID 1465231) Laurent Vivier
2021-11-05 17:01 ` Peter Maydell
@ 2021-11-08 8:06 ` Mark Cave-Ayland
1 sibling, 0 replies; 5+ messages in thread
From: Mark Cave-Ayland @ 2021-11-08 8:06 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Peter Maydell
On 05/11/2021 16:52, Laurent Vivier wrote:
> Rewrite the function using g_string_append_printf() rather than
> g_strdup_printf()/g_strconcat().
>
> Fixes: df8abbbadf74 ("macfb: add common monitor modes supported by the MacOS toolbox ROM")
> Cc: mark.cave-ayland@ilande.co.uk
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> hw/display/macfb.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index 4b352eb89c3f..277d3e663331 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -440,21 +440,18 @@ static MacFbMode *macfb_find_mode(MacfbDisplayType display_type,
>
> static gchar *macfb_mode_list(void)
> {
> - gchar *list = NULL;
> - gchar *mode;
> + GString *list = g_string_new("");
> MacFbMode *macfb_mode;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(macfb_mode_table); i++) {
> macfb_mode = &macfb_mode_table[i];
>
> - mode = g_strdup_printf(" %dx%dx%d\n", macfb_mode->width,
> + g_string_append_printf(list, " %dx%dx%d\n", macfb_mode->width,
> macfb_mode->height, macfb_mode->depth);
> - list = g_strconcat(mode, list, NULL);
> - g_free(mode);
> }
>
> - return list;
> + return g_string_free(list, FALSE);
> }
>
>
> @@ -643,7 +640,7 @@ static bool macfb_common_realize(DeviceState *dev, MacfbState *s, Error **errp)
> gchar *list;
> error_setg(errp, "unknown display mode: width %d, height %d, depth %d",
> s->width, s->height, s->depth);
> - list = macfb_mode_list();
> + list = macfb_mode_list();
> error_append_hint(errp, "Available modes:\n%s", list);
> g_free(list);
Thanks Laurent, looks good to me.
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-08 8:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-05 16:52 [PATCH] macfb: fix a memory leak (CID 1465231) Laurent Vivier
2021-11-05 17:01 ` Peter Maydell
2021-11-05 18:32 ` Laurent Vivier
2021-11-05 18:43 ` Peter Maydell
2021-11-08 8:06 ` Mark Cave-Ayland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).