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

test cache page allocation failure #464

Merged
merged 3 commits into from
Oct 7, 2024
Merged

Conversation

rrrrrrmb
Copy link
Contributor

@rrrrrrmb rrrrrrmb commented Oct 7, 2024

This PR solves #442. This test creates an entry structure to initialize the cache and then creates more structures to fill the cache until a new page allocation fails.

@goatshriek goatshriek added the testing pertains to the test suite label Oct 7, 2024
@goatshriek goatshriek self-assigned this Oct 7, 2024
Copy link

codecov bot commented Oct 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.59%. Comparing base (afdc224) to head (26a252e).
Report is 2 commits behind head on latest.

Additional details and impacted files
@@            Coverage Diff             @@
##           latest     #464      +/-   ##
==========================================
+ Coverage   90.54%   90.59%   +0.04%     
==========================================
  Files          47       47              
  Lines        4379     4379              
  Branches      587      587              
==========================================
+ Hits         3965     3967       +2     
+ Misses        280      279       -1     
+ Partials      134      133       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@goatshriek goatshriek left a comment

Choose a reason for hiding this comment

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

Your test works perfectly, and has in fact uncovered a memory leak!

The result of realloc_mem on line 70 of src/cache.c isn't dealt with properly in the case of failure. The easiest solution here would be to move the assignment currently on line 83 to before the alloc call, so that the cache has the new structure even in the case of an error. If you'd like to give that a shot please do, or if you see a better solution you can try that instead.

this updates cache structure before the alloc_mem call so that the
structure is intact even in case of an allocation failure
@rrrrrrmb
Copy link
Contributor Author

rrrrrrmb commented Oct 7, 2024

Thanks for the hint ! I've made the suggested change and looks like that fixed it :)

@goatshriek goatshriek merged commit 1d2df58 into goatshriek:latest Oct 7, 2024
56 checks passed
@goatshriek goatshriek added bug something is broken or missing hacktoberfest-accepted accepted work that is valid for hacktoberfest labels Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is broken or missing hacktoberfest-accepted accepted work that is valid for hacktoberfest testing pertains to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants