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

dbfopen: fix possible memory leaks when using realloc #166

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ymdatta
Copy link

@ymdatta ymdatta commented Oct 29, 2024

Fixes #165

@ymdatta
Copy link
Author

ymdatta commented Oct 29, 2024

I found the build process very confusing and instructions from README were I believe of older versions. I confess that my autoconf knowledge isn't great. Had to try lot of things before able to build the repository. Maybe, I'll open a different PR for that.

Tests done:

  1. Ran cppcheck on changes, previous issue was resolved.
  2. Ran make check and tests passed.

Let me know if there are any more tests I can do. Thanks.

dbfopen.c Outdated Show resolved Hide resolved
dbfopen.c Outdated Show resolved Hide resolved
dbfopen.c Outdated Show resolved Hide resolved
dbfopen.c Outdated Show resolved Hide resolved
@ymdatta ymdatta force-pushed the master branch 2 times, most recently from 80f78f7 to b7432a6 Compare November 1, 2024 17:25
@ymdatta
Copy link
Author

ymdatta commented Nov 1, 2024

Changes:

  1. Deleted all the free() on fields of DBFhandle. When a realloc error is seen, decreasing number of fields by 1 and returning.
  2. As part of DBFDeleteField, delete reallocs as suggested. I am bit unsure if I did this correctly or not. I may want some eyes on this @rouault .

dbfopen.c Show resolved Hide resolved
dbfopen.c Outdated Show resolved Hide resolved
s1.out Outdated Show resolved Hide resolved
char *, realloc(psDBF->pszHeader, psDBF->nFields * XBASE_FLDHDR_SZ));

if (!psDBF_char_realloc_ptr)
{
return FALSE;
Copy link
Member

Choose a reason for hiding this comment

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

there would be much more work to restore the psDBF state to a consistente state... I'm afraid that the best in that method would be to just "assert(false); return FALSE;" on failed memory allocations. Probably the same in DBFAlterFieldDefn()

dbfopen.c Outdated Show resolved Hide resolved
@rouault
Copy link
Member

rouault commented Nov 1, 2024

@thbeu your review would be appreciated

Co-authored-by: Even Rouault <[email protected]>
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.

dbfopen: possible memory leak with realloc when allocation fails
2 participants