On Thu, 24 Aug 2023 at 17:28, Dmitry Frolov <fro...@swemel.ru> wrote:
>
> Fuzzing causes thousands of identical crashes with message:
> "AddressSanitizer: 3744 byte(s) leaked in 1 allocation(s)"
>
> Fixes: 060ab76356 ("gtk: don't exit early in case gtk init fails")
>
> Signed-off-by: Dmitry Frolov <fro...@swemel.ru>
> ---
> ui/gtk.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 8ba41c8f13..996ca7949d 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -2358,6 +2358,10 @@ static gboolean gtkinit;
>
> static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
> {
> + if (!gtkinit) {
> + fprintf(stderr, "gtk initialization failed\n");
> + exit(1);
> + }
> VirtualConsole *vc;
This breaks our rule against having variable declarations
in the middle of code blocks. The variable declarations
need to come first, before this code.
More generally, I don't understand why this change is
necessary. If gtkinit is false, we're going to call
exit(), which will clean up all our allocations. The
specific allocation of the GtkDisplayState can hardly
be the only one that we still have allocated and
are relying on the cleanup-on-exit for.
> GtkDisplayState *s = g_malloc0(sizeof(*s));
> @@ -2365,10 +2369,6 @@ static void gtk_display_init(DisplayState *ds,
> DisplayOptions *opts)
> GtkIconTheme *theme;
> char *dir;
>
> - if (!gtkinit) {
> - fprintf(stderr, "gtk initialization failed\n");
> - exit(1);
> - }
> assert(opts->type == DISPLAY_TYPE_GTK);
> s->opts = opts;
thanks
-- PMM
