Skip to content
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

Fix memleak when writer is closed prior to closing containers #303

Merged
merged 1 commit into from
Jul 14, 2022
Merged

Fix memleak when writer is closed prior to closing containers #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions ionc/ion_writer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2812,6 +2812,7 @@ iERR _ion_writer_free(ION_WRITER *pwriter)
// Free the writer's members.
UPDATEERROR(_ion_writer_free_local_symbol_table( pwriter ));
UPDATEERROR( _ion_writer_free_temp_pool( pwriter ));
UPDATEERROR(_ion_writer_free_pending_pool(pwriter));

// If the writer allocated the stream, free it.
if (pwriter->writer_owns_stream) {
Expand Down Expand Up @@ -3281,3 +3282,14 @@ iERR _ion_writer_free_temp_pool( ION_WRITER *pwriter )

iRETURN;
}

iERR _ion_writer_free_pending_pool(ION_WRITER *writer) {
iENTER;

if (writer->_pending_temp_entity_pool != NULL) {
ion_free_owner(writer->_pending_temp_entity_pool);
writer->_pending_temp_entity_pool = NULL;
}

RETURN(__location_name__, __line__, __count__++, err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an intentional choice not to use iRETURN?

Should this just be a void function since it looks like it can't return anything but IERR_OK?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea that was intentional. Since the fail label that iRETURN defines isn't used, using it over RETURN only adds to the warnings generated at build.

It could definitely be a void function. I maintained the signature that the other free functions had for writer members, but all of them could be converted to void since all 3 only return IERR_OK.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. So maybe we need a separate (and larger) task to get rid of unnecessary iRETURN usages, if we want to reduce warnings. Approving this PR.

}
1 change: 1 addition & 0 deletions ionc/ion_writer_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ iERR ion_temp_buffer_reset(ION_TEMP_BUFFER *temp_buffer);
iERR _ion_writer_allocate_temp_pool( ION_WRITER *pwriter );
iERR _ion_writer_reset_temp_pool( ION_WRITER *pwriter );
iERR _ion_writer_free_temp_pool( ION_WRITER *pwriter );
iERR _ion_writer_free_pending_pool( ION_WRITER *pwriter );



Expand Down