commit db18bccd00d927df1826fc34085852caad94a3c3
parent 042da9c7f7790d50d94234fdbe7788bad8d0895d
Author: Eduardo Fontana Lazzari <edufonlaz@gmail.com>
Date: Mon, 3 Mar 2025 13:40:27 +0100
Fix bugs and memory leaks
Add a check for the res of the return of parsing functions for
substructures such as volume, source, sensor and surface;
Ensure the memory is freed if there is an error during the parsing of
these structs.
Diffstat:
6 files changed, 59 insertions(+), 9 deletions(-)
diff --git a/src/sphin_config.c b/src/sphin_config.c
@@ -80,8 +80,8 @@ release_config(ref_T* address)
struct sphin_config* config = NULL;
struct sphin* sphin = NULL;
size_t i, nvolumes, nsurfaces;
- struct sphin_volume** volumes;
- struct sphin_surface** surfaces;
+ struct sphin_volume** volumes = NULL;
+ struct sphin_surface** surfaces = NULL;
ASSERT(NULL != address);
config = CONTAINER_OF(address, struct sphin_config, ref);
@@ -136,6 +136,7 @@ load_stream
ASSERT(NULL != streamname);
ASSERT(NULL != out_config);
+
str_init(sphin->allocator, &line);
res = config_create(sphin, &config);
diff --git a/src/sphin_geometry.c b/src/sphin_geometry.c
@@ -26,6 +26,7 @@
#include "sphin_c.h"
#include "sphin_geometry.h"
+#include <rsys/cstr.h>
#include <star/sstl.h>
/*******************************************************************************
@@ -65,6 +66,9 @@ geometry_create
geom = MEM_CALLOC(sphin->allocator, 1, sizeof(struct sphin_geometry));
if (NULL == geom) { res = RES_MEM_ERR; goto error; }
+
+
+ /* Init geometry ref counter and init member variables */
ref_init(&geom->ref);
SPHIN(ref_get(sphin));
geom->sphin = sphin;
@@ -107,11 +111,19 @@ geometry_parse
double* coords = NULL;
size_t* indices = NULL;
- /* Create the sstl device with the same allocator and logger as the sphin
- * handler. TODO Comment*/
+ /* Create the SSTL device using the same allocator and logger as the sphin
+ * handler. We intentionally instantiate and free the SSTL struct for
+ * each geometry, rather than creating it once at the sphin_config level.
+ * We argue that the performance loss caused by this strategy is marginal
+ * compared to the advantage of keeping the effects of this structure
+ * encapsulated to the scope of this function. */
res = sstl_create(sphin->logger, sphin->allocator, 1, &sstl);
if (RES_OK != res) { goto error; }
+ /* Parse side */
+ side = strtok_r(value, " \t", &token_ptr);
+ if (NULL == side){ res = RES_BAD_ARG; goto error; }
+
/* Parse filename */
filename = strtok_r(NULL, " \t", &token_ptr);
if (NULL == filename){ res = RES_BAD_ARG; goto error; }
@@ -122,8 +134,7 @@ geometry_parse
coords = darray_double_data_get(&geom->coords);
indices = darray_size_t_data_get(&geom->indices); /* Parse side */
- side = strtok_r(value, " \t", &token_ptr);
- if (NULL == side){ res = RES_BAD_ARG; goto error; }
+
if (0 == strcmp(side, "FRONT")){ geom->side = SPHIN_SIDE_FRONT; }
else if (0 == strcmp(side, "BACK")){ geom->side = SPHIN_SIDE_BACK; }
else { res = RES_BAD_ARG; goto error; }
diff --git a/src/sphin_sensor.c b/src/sphin_sensor.c
@@ -87,7 +87,6 @@ release_sensor(ref_T* address)
struct sphin* sphin = NULL;
ASSERT(NULL != address);
-
sensor = CONTAINER_OF(address, struct sphin_sensor, ref);
str_release(&sensor->name);
sphin = sensor->sphin;
@@ -150,6 +149,8 @@ parse_sensor
res = sensor_create(sphin, name, &sensor);
if (RES_OK != res) { goto error; }
+ res = txtrdr_read_line(txtrdr);
+ if (RES_OK != res) { goto error; }
while (NULL != txtrdr_get_line(txtrdr)) {
res = str_set(&line, txtrdr_get_cline(txtrdr));
@@ -172,12 +173,13 @@ parse_sensor
else {
break;
}
+ if (RES_OK != res) { goto error; }
/* Advance one line in the parsing */
res = txtrdr_read_line(txtrdr);
if (RES_OK != res) { goto error; }
}
-
exit:
+ str_release(&line);
*out_sensor = sensor;
return res;
error:
diff --git a/src/sphin_source.c b/src/sphin_source.c
@@ -267,6 +267,7 @@ parse_source
else {
break;
}
+ if (RES_OK != res) { goto error; }
/* Advance one line in the parsing */
res = txtrdr_read_line(txtrdr);
if (RES_OK != res) { goto error; }
diff --git a/src/sphin_surface.c b/src/sphin_surface.c
@@ -94,8 +94,10 @@ error:
static void
release_surface(ref_T* address)
{
+ size_t i, ngeometries;
struct sphin* sphin = NULL;
struct sphin_brdf* brdf = NULL;
+ struct sphin_geometry** geometries= NULL;
struct sphin_sensor* sensor =NULL;
struct sphin_source* source = NULL;
struct sphin_surface* surface = NULL;
@@ -103,7 +105,18 @@ release_surface(ref_T* address)
surface = CONTAINER_OF(address, struct sphin_surface, ref);
str_release(&surface->name);
+
+ /* Retrieve the number of geometries associated with the volume and decrease
+ * the reference counter of each one of them */
+ ngeometries = darray_sphin_geometry_ptr_size_get(&surface->geometries);
+ geometries = darray_sphin_geometry_ptr_data_get(&surface->geometries);
+
+ /* Put references for each one of the geometries */
+ for (i=ngeometries; i; i--) {
+ SPHIN(geometry_ref_put(geometries[i-1]));
+ };
darray_sphin_geometry_ptr_release(&surface->geometries);
+
sphin = surface->sphin;
brdf = surface->brdf;
sensor = surface->sensor;
@@ -205,6 +218,7 @@ parse_surface
else {
break;
}
+ if (RES_OK != res) { goto error; }
/* Advance one line in the parsing */
res = txtrdr_read_line(txtrdr);
if (RES_OK != res) { goto error; }
@@ -218,6 +232,10 @@ exit:
str_release(&line);
return res;
error:
+ if (NULL != surface) {
+ SPHIN(surface_ref_put(surface));
+ surface = NULL;
+ }
goto exit;
}
diff --git a/src/sphin_volume.c b/src/sphin_volume.c
@@ -90,6 +90,8 @@ error:
static void
release_volume(ref_T* address)
{
+ size_t i, ngeometries;
+ struct sphin_geometry** geometries = NULL;
struct sphin* sphin = NULL;
struct sphin_sensor* sensor = NULL;
struct sphin_volume* volume = NULL;
@@ -97,7 +99,18 @@ release_volume(ref_T* address)
volume = CONTAINER_OF(address, struct sphin_volume, ref);
str_release(&volume->name);
+
+ /* Retrieve the number of geometries associated with the volume and decrease
+ * the reference counter of each one of them */
+ ngeometries = darray_sphin_geometry_ptr_size_get(&volume->geometries);
+ geometries = darray_sphin_geometry_ptr_data_get(&volume->geometries);
+
+ /* Put references for each one of the geometries */
+ for (i=ngeometries; i; i--) {
+ SPHIN(geometry_ref_put(geometries[i-1]));
+ };
darray_sphin_geometry_ptr_release(&volume->geometries);
+
sphin = volume->sphin;
sensor = volume->sensor;
MEM_RM(sphin->allocator, volume);
@@ -213,7 +226,6 @@ parse_volume
while (NULL != txtrdr_get_line(txtrdr)) {
res = str_set(&line, txtrdr_get_cline(txtrdr));
if (RES_OK != res) { goto error; }
-
/* parse keyword */
token = strtok_r(str_get(&line), ":", &token_ptr);
if (NULL == token){ res = RES_BAD_ARG; goto error; }
@@ -236,6 +248,7 @@ parse_volume
else {
break;
}
+ if (RES_OK != res) { goto error; }
/* Advance one line in the parsing */
res = txtrdr_read_line(txtrdr);
if (RES_OK != res) { goto error; }
@@ -249,6 +262,10 @@ exit:
str_release(&line);
return res;
error:
+ if (NULL != volume) {
+ SPHIN(volume_ref_put(volume));
+ volume = NULL;
+ }
goto exit;
}
