Skip to content

Fix memory leaks #2422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cary-ilm merged 3 commits into from
May 17, 2026
+59 −57
Merged

Fix memory leaks #2422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a16be8c
fix memory leak in test only - invalid api call
kdt3rd May 17, 2026
92eccb5
simplify string init
kdt3rd May 17, 2026
0384e06
refactor part memory handling for parts
kdt3rd May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 23 additions & 36 deletions src/lib/OpenEXRCore/internal_structs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,26 +162,22 @@ internal_exr_destroy_part (exr_context_t ctxt, exr_priv_part_t cur)
atomic_store (&(cur->chunk_table), (uintptr_t) (0));
#endif
if (ctable && ((uintptr_t) ctable) != UINTPTR_MAX) dofree (ctable);

/* we avoid malloc on one part files, but need to check */
if (cur != &(ctxt->first_part)) { dofree (cur); }
else { memset (cur, 0, sizeof (struct _priv_exr_part_t)); }
}

/**************************************/

static void
internal_exr_destroy_parts (exr_context_t ctxt)
{
exr_memory_free_func_t dofree = ctxt->free_fn;
for (int p = 0; p < ctxt->num_parts; ++p)
{
exr_priv_part_t cur = ctxt->parts[p];

internal_exr_destroy_part (ctxt, cur);

/* the first one is always the one that is part of the file */
if (cur != &(ctxt->first_part)) { dofree (cur); }
else { memset (cur, 0, sizeof (struct _priv_exr_part_t)); }
}
internal_exr_destroy_part (ctxt, ctxt->parts[p]);

if (ctxt->num_parts > 1) dofree (ctxt->parts);
if (ctxt->parts != &(ctxt->init_part)) ctxt->free_fn (ctxt->parts);
ctxt->init_part = NULL;
ctxt->parts = NULL;
ctxt->num_parts = 0;
}
Expand All @@ -200,15 +196,14 @@ internal_exr_add_part (

if (ncount == 1)
{
/* no need to zilch, the parent struct will have already been zero'ed */
/* avoid an initial malloc by embedding a part in the struct... */
part = &(f->first_part);
/* also need a pointer to pointer... */
f->init_part = part;
nptrs = &(f->init_part);
}
else
{
struct _priv_exr_part_t nil = {0};

part = f->alloc_fn (sizeof (struct _priv_exr_part_t));
if (!part) return f->standard_error (f, EXR_ERR_OUT_OF_MEMORY);

Expand All @@ -218,8 +213,8 @@ internal_exr_add_part (
f->free_fn (part);
return f->standard_error (f, EXR_ERR_OUT_OF_MEMORY);
}
*part = nil;
}
memset (part, 0, sizeof (struct _priv_exr_part_t));

/* assign appropriately invalid values */
part->storage_mode = EXR_STORAGE_LAST_TYPE;
Expand All @@ -234,16 +229,13 @@ internal_exr_add_part (
part->dwa_compression_level = f->default_dwa_quality;

/* put it into the part table */
if (ncount > 1)
for (int p = 0; p < f->num_parts; ++p)
{
for (int p = 0; p < f->num_parts; ++p)
{
nptrs[p] = f->parts[p];
}
nptrs[ncount - 1] = part;
nptrs[p] = f->parts[p];
}
nptrs[f->num_parts] = part;

if (f->num_parts > 1) { f->free_fn (f->parts); }
if (f->parts && f->parts != &(f->init_part)) { f->free_fn (f->parts); }
f->parts = nptrs;
f->num_parts = ncount;
if (outpart) *outpart = part;
Expand All @@ -264,20 +256,8 @@ internal_exr_revert_add_part (
*new_index = -1;

internal_exr_destroy_part (ctxt, part);
if (ncount == 0)
{
ctxt->num_parts = 0;
ctxt->init_part = NULL;
ctxt->parts = NULL;
}
else if (ncount == 1)
{
if (part == &(ctxt->first_part)) ctxt->first_part = *(ctxt->parts[1]);
ctxt->init_part = &(ctxt->first_part);
ctxt->free_fn (ctxt->parts);
ctxt->parts = &(ctxt->init_part);
}
else

if ( ncount > 0 )
{
int np = 0;
for (int p = 0; p < ctxt->num_parts; ++p)
Expand All @@ -287,6 +267,13 @@ internal_exr_revert_add_part (
++np;
}
}
else
{
if (ctxt->parts && ctxt->parts != &(ctxt->init_part))
ctxt->free_fn (ctxt->parts);

ctxt->parts = NULL;
}
ctxt->num_parts = ncount;
}

Expand Down
10 changes: 4 additions & 6 deletions src/lib/OpenEXRCore/part.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,16 @@ exr_add_part (
(uint64_t) pnamelen));
}

rv = internal_exr_add_part (ctxt, &part, new_index);
if (rv != EXR_ERR_SUCCESS) return EXR_UNLOCK_AND_RETURN (rv);

if (ctxt->num_parts > 0)
{
// ensure multi part has at least some name?
if (!partname) partname = "";

for ( int pidx = 0; pidx < ctxt->num_parts - 1; ++pidx )
for ( int pidx = 0; pidx < ctxt->num_parts; ++pidx )
{
const exr_attribute_t* pname = ctxt->parts[pidx]->name;
if (!pname)
{
internal_exr_revert_add_part (ctxt, &part, new_index);
return EXR_UNLOCK_AND_RETURN (
ctxt->print_error (
ctxt,
Expand All @@ -106,7 +102,6 @@ exr_add_part (
}
if (!strcmp (partname, pname->string->str))
{
internal_exr_revert_add_part (ctxt, &part, new_index);
return EXR_UNLOCK_AND_RETURN (
ctxt->print_error (
ctxt,
Expand All @@ -117,6 +112,9 @@ exr_add_part (
}
}

rv = internal_exr_add_part (ctxt, &part, new_index);
if (rv != EXR_ERR_SUCCESS) return EXR_UNLOCK_AND_RETURN (rv);

part->storage_mode = type;
switch (type)
{
Expand Down
28 changes: 17 additions & 11 deletions src/lib/OpenEXRCore/string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
exr_result_t
exr_attr_string_init (exr_context_t ctxt, exr_attr_string_t* s, int32_t len)
{
exr_attr_string_t nil = {0};
if (!ctxt) return EXR_ERR_MISSING_CONTEXT_ARG;

if (len < 0)
Expand All @@ -31,10 +30,10 @@ exr_attr_string_init (exr_context_t ctxt, exr_attr_string_t* s, int32_t len)
EXR_ERR_INVALID_ARGUMENT,
"Invalid reference to string object to initialize");

*s = nil;
s->str = (char*) ctxt->alloc_fn ((size_t) (len + 1));
if (s->str == NULL)
return ctxt->standard_error (ctxt, EXR_ERR_OUT_OF_MEMORY);

s->length = len;
s->alloc_size = len + 1;
return EXR_ERR_SUCCESS;
Expand All @@ -46,7 +45,6 @@ exr_result_t
exr_attr_string_init_static_with_length (
exr_context_t ctxt, exr_attr_string_t* s, const char* v, int32_t len)
{
exr_attr_string_t nil = {0};
if (!ctxt) return EXR_ERR_MISSING_CONTEXT_ARG;

if (len < 0)
Expand All @@ -68,9 +66,9 @@ exr_attr_string_init_static_with_length (
EXR_ERR_INVALID_ARGUMENT,
"Invalid reference to string object to initialize");

*s = nil;
s->length = len;
s->str = v;
s->length = len;
s->alloc_size = 0;
s->str = v;
return EXR_ERR_SUCCESS;
}

Expand Down Expand Up @@ -132,7 +130,11 @@ exr_attr_string_create_with_length (
# pragma GCC diagnostic ignored "-Wstringop-truncation"
#endif
if (d)
strncpy (outs, d, (size_t) len);
{
size_t slen = strnlen( d, (size_t) len);
memcpy (outs, d, slen);
memset (outs + slen, '\0', ((size_t)len - slen) + 1);
}
else
memset (outs, 0, (size_t) len);
#ifdef _MSC_VER
Expand All @@ -143,7 +145,7 @@ exr_attr_string_create_with_length (
# pragma GCC diagnostic pop
#endif
}
outs[len] = '\0';
else outs[0] = '\0';
}
return rv;
}
Expand Down Expand Up @@ -210,9 +212,13 @@ exr_attr_string_set_with_length (
# pragma GCC diagnostic ignored "-Wstringop-truncation"
#endif
if (d)
strncpy (sstr, d, (size_t) len);
{
size_t slen = strnlen( d, (size_t) len);
memcpy (sstr, d, slen);
memset (sstr + slen, '\0', ((size_t)len - slen) + 1);
}
else
memset (sstr, 0, (size_t) len);
memset (sstr, 0, (size_t) (len + 1));
#ifdef _MSC_VER
# pragma warning(pop)
#elif defined(__clang__)
Expand All @@ -221,7 +227,7 @@ exr_attr_string_set_with_length (
# pragma GCC diagnostic pop
#endif
}
sstr[len] = '\0';
else sstr[0] = '\0';
return EXR_ERR_SUCCESS;
}
exr_attr_string_destroy (ctxt, s);
Expand Down
1 change: 0 additions & 1 deletion src/test/OpenEXRCoreTest/general_attr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,6 @@ testBytesHelper (exr_context_t f)

EXRCORE_TEST_RVAL (exr_attr_bytes_init (f, &b, 0, 0));
EXRCORE_TEST_RVAL (exr_attr_bytes_destroy (f, &b));
EXRCORE_TEST_RVAL (exr_attr_bytes_init (f, &b, 0, 0));
EXRCORE_TEST_RVAL (exr_attr_bytes_create (f, &b, 0, 4, NULL, data4));
EXRCORE_TEST_RVAL (exr_attr_bytes_copy (f, &b2, &b));
EXRCORE_TEST_RVAL (exr_attr_bytes_destroy (f, &b2));
Expand Down
18 changes: 15 additions & 3 deletions src/test/OpenEXRCoreTest/write.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,20 @@ testStartWriteScan (const std::string& tempdir)
EXRCORE_TEST_RVAL (
exr_add_part (outf, "beauty", EXR_STORAGE_SCANLINE, &partidx));
EXRCORE_TEST (partidx == 0);
EXRCORE_TEST_RVAL (exr_get_count (outf, &partidx));
/* dup name check */
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_INVALID_ARGUMENT,
exr_add_part (outf, "beauty", EXR_STORAGE_SCANLINE, &partidx));
/* this add should work, making 2 parts */
EXRCORE_TEST_RVAL (
exr_add_part (outf, "other", EXR_STORAGE_SCANLINE, &partidx));
EXRCORE_TEST (partidx == 1);
/* test repeated add failure doesn't leak */
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_INVALID_ARGUMENT,
exr_add_part (outf, "beauty", EXR_STORAGE_SCANLINE, &partidx));
EXRCORE_TEST_RVAL (exr_get_count (outf, &partidx));
EXRCORE_TEST (partidx == 2);
partidx = 0;

exr_chunk_info_t cinfo;
Expand All @@ -105,7 +117,7 @@ testStartWriteScan (const std::string& tempdir)
exr_get_name (outf, partidx - 1, &partname));
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_ARGUMENT_OUT_OF_RANGE,
exr_get_name (outf, partidx + 1, &partname));
exr_get_name (outf, partidx + 2, &partname));
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_INVALID_ARGUMENT, exr_get_name (outf, partidx, NULL));
EXRCORE_TEST_RVAL (exr_get_name (outf, partidx, &partname));
Expand All @@ -119,7 +131,7 @@ testStartWriteScan (const std::string& tempdir)
exr_get_storage (outf, partidx - 1, &storage));
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_ARGUMENT_OUT_OF_RANGE,
exr_get_storage (outf, partidx + 1, &storage));
exr_get_storage (outf, partidx + 2, &storage));
EXRCORE_TEST_RVAL_FAIL (
EXR_ERR_INVALID_ARGUMENT, exr_get_storage (outf, partidx, NULL));
EXRCORE_TEST_RVAL (exr_get_storage (outf, partidx, &storage));
Expand Down
Loading