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.

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

Patch from Dave Bremer #306

Merged
merged 1 commit into from
May 25, 2023
Merged

Conversation

markcmiller86
Copy link
Member

@markcmiller86 markcmiller86 commented Apr 13, 2023

Resolves #221

Fixes poorly designed freeing of file scope gloabls. Thanks to Dave Bremer.

While doing some testing, I found a memory error in DBClose(). I made a patch and wanted to offer it for inclusion in the Silo source. It's a two-line change, so I could hand it to you directly, or submit it through GitHub.

Current code:

        retval = (dbfile->pub.close) (dbfile);
        free(dbfile->pub.file_scope_globals);

Proposed change:

        SILO_Globals_t *tmp_file_scope_globals = dbfile->pub.file_scope_globals;
        retval = (dbfile->pub.close) (dbfile);
        free(tmp_file_scope_globals);

The dbfile->pub.close() function tends to call free on dbfile, so the pointer at dbfile->pub.file_scope_globals is in freed memory. It tends to work, unless you use a Windows debug build, or if you set MALLOC_PERTURB_ on Linux, which writes a value into a freed buffe

Fixes poorly designed freeing of file scope gloabls. Thanks to Dave Bremer.
@markcmiller86 markcmiller86 merged commit b0e202d into 4.11RC May 25, 2023
markcmiller86 added a commit that referenced this pull request May 25, 2023
markcmiller86 added a commit that referenced this pull request May 25, 2023
…globals

Merge pull request #306 from LLNL/bug-mcm86-13apr23-free-file-globals
@markcmiller86 markcmiller86 deleted the bug-mcm86-13apr23-free-file-globals branch May 25, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant