Mercurial > molko
changeset 238:b30c3af37a01
rpg: fix possible dangling pointer in tileset-file
We were referencing a pointer in a loop that reallocate the array, create a
second loop to generate the second tileset_animation array.
author | David Demelier <markand@malikania.fr> |
---|---|
date | Fri, 27 Nov 2020 13:32:52 +0100 |
parents | 1bf5bd306bb0 |
children | d47e70da760e |
files | librpg/rpg/tileset-file.c |
diffstat | 1 files changed, 47 insertions(+), 49 deletions(-) [+] |
line wrap: on
line diff
--- a/librpg/rpg/tileset-file.c Fri Nov 27 13:31:27 2020 +0100 +++ b/librpg/rpg/tileset-file.c Fri Nov 27 13:32:52 2020 +0100 @@ -43,9 +43,9 @@ * structure. * * As animations require a texture and a sprite to be present, we need to store - * them locally in the tileset structure. + * them locally in the tileset_file structure. * - * tileset_file->anims[0] array (struct anim): + * tileset_file->anims[0] array (struct tileset_animation_block): * * [0] [1] [N] * | texture | texture | texture @@ -61,10 +61,10 @@ * The second array is the exposed array through the tileset->anims pointer, * animations are referenced from the first array. This is because user may need * or replace the tileset by itself and as such we need to keep track of the - * resource the tileset_file have allocated itself. + * resource the tileset_file has allocated itself. */ -struct anim { +struct tileset_animation_block { struct texture texture; struct sprite sprite; struct animation animation; @@ -89,6 +89,14 @@ unsigned int ncolumns; }; +static void +tileset_animation_block_finish(void *data) +{ + struct tileset_animation_block *anim = data; + + texture_finish(&anim->texture); +} + static int tileset_tiledef_cmp(const void *d1, const void *d2) { @@ -142,14 +150,13 @@ short x, y; unsigned short id, w, h; - struct tileset_file *tf = ctx->tf; struct tileset_tiledef *td; - if (!alloc_pool_init(&tf->tiledefs, sizeof (*td), NULL)) + if (!alloc_pool_init(&ctx->tf->tiledefs, sizeof (*td), NULL)) return false; while (fscanf(ctx->fp, "%hu|%hd|%hd|%hu|%hu\n", &id, &x, &y, &w, &h) == 5) { - if (!(td = alloc_pool_new(&tf->tiledefs))) + if (!(td = alloc_pool_new(&ctx->tf->tiledefs))) return false; td->id = id; @@ -159,24 +166,14 @@ td->h = h; } - /* - * Sort the array and expose it through the tileset->tiledefs pointer. - */ - qsort(tf->tiledefs.data, tf->tiledefs.size, tf->tiledefs.elemsize, tileset_tiledef_cmp); - ctx->tileset->tiledefs = tf->tiledefs.data; - ctx->tileset->tiledefsz = tf->tiledefs.size; + /* Sort the array and expose it through the tileset->tiledefs pointer. */ + qsort(ctx->tf->tiledefs.data, ctx->tf->tiledefs.size, ctx->tf->tiledefs.elemsize, tileset_tiledef_cmp); + ctx->tileset->tiledefs = ctx->tf->tiledefs.data; + ctx->tileset->tiledefsz = ctx->tf->tiledefs.size; return true; } -static void -anim_finish(void *data) -{ - struct anim *anim = data; - - texture_finish(&anim->texture); -} - static bool parse_animations(struct context *ctx, const char *line) { @@ -184,34 +181,37 @@ unsigned short id; unsigned int delay; - char filename[FILENAME_MAX + 1], path[PATH_MAX]; - struct tileset *tileset = ctx->tileset; - struct tileset_file *tf = ctx->tf; - struct tileset_animation *ta; - struct anim *anim; + char filename[FILENAME_MAX + 1]; - if (!alloc_pool_init(&tf->anims[0], sizeof (*anim), anim_finish) || - !alloc_pool_init(&tf->anims[1], sizeof (*ta), NULL)) + if (!alloc_pool_init(&ctx->tf->anims[0], sizeof (struct tileset_animation_block), tileset_animation_block_finish) || + !alloc_pool_init(&ctx->tf->anims[1], sizeof (struct tileset_animation), NULL)) return false; + /* + * 1. Create the first array of animation, sprite and texture that are + * owned by the tileset_file structure. + */ while (fscanf(ctx->fp, "%hu|" MAX_F(FILENAME_MAX) "|%u", &id, filename, &delay) == 3) { - if (!(anim = alloc_pool_new(&tf->anims[0]))) + struct tileset_animation_block *anim; + + if (!(anim = alloc_pool_new(&ctx->tf->anims[0]))) + return false; + if (!image_open(&anim->texture, pprintf("%s/%s", ctx->basedir, filename))) return false; - snprintf(path, sizeof (path), "%s/%s", ctx->basedir, filename); - - if (!image_open(&anim->texture, path)) - return false; - - /* Initialize animation. */ sprite_init(&anim->sprite, &anim->texture, ctx->tilewidth, ctx->tileheight); animation_init(&anim->animation, &anim->sprite, delay); + } - /* - * Now create the real tileset_animation visible in - * tileset->anims. - */ - if (!(ta = alloc_pool_new(&tf->anims[1]))) + /* + * 2. Create the second array that only consist of pointers to + * animations referencing the first array. + */ + for (size_t i = 0; i < ctx->tf->anims[0].size; ++i) { + struct tileset_animation_block *anim = alloc_pool_get(&ctx->tf->anims[0], i); + struct tileset_animation *ta; + + if (!(ta = alloc_pool_new(&ctx->tf->anims[1]))) return false; ta->id = id; @@ -219,12 +219,12 @@ } /* - * Sort and expose the animation array through the tileset->animsz - * pointer. + * 3. Finally expose the second array through the tileset->anims pointer + * and sort it. */ - qsort(tf->anims[1].data, tf->anims[1].size, tf->anims[1].elemsize, tileset_animation_cmp); - tileset->anims = tf->anims[1].data; - tileset->animsz = tf->anims[1].size; + qsort(ctx->tf->anims[1].data, ctx->tf->anims[1].size, ctx->tf->anims[1].elemsize, tileset_animation_cmp); + ctx->tileset->anims = ctx->tf->anims[1].data; + ctx->tileset->animsz = ctx->tf->anims[1].size; return true; } @@ -232,16 +232,14 @@ static bool parse_image(struct context *ctx, const char *line) { - char path[PATH_MAX], *p; + char *p; if (ctx->tilewidth == 0 || ctx->tileheight == 0) return errorf("missing tile dimensions before image"); if (!(p = strchr(line, '|'))) return errorf("could not parse image"); - snprintf(path, sizeof (path), "%s/%s", ctx->basedir, p + 1); - - if (!image_open(&ctx->tf->image, path)) + if (!image_open(&ctx->tf->image, pprintf("%s/%s", ctx->basedir, p + 1))) return false; sprite_init(&ctx->tf->sprite, &ctx->tf->image, ctx->tilewidth, ctx->tileheight);