search

goatshriek / stumpless

a C logging library built for performance and features
https://goatshriek.github.io/stumpless
Apache License 2.0
444 stars 331 forks source link

test cache free of non-cache memory #441

Open goatshriek opened 2 weeks ago

goatshriek commented 2 weeks ago

Stumpless uses a simple slab cache implementation in src/cache.c to consolidate memory allocations for common structures like entries. Test coverage of this cache is fairly complete, but still has some gaps.

One such gap is what happens when free is called on an object that is not owned by the cache. The mutex for the cache should be unlocked once this routine is finished, but currently this is an untested behavior. You will add a test for this case.

General Approach

There are a few details left out of the following approach, for you to fill in as you encounter them. If you find you need help, please ask here or on the project gitter and someone can help you get past the stumbling block.

First, familiarize yourself with the cache_free implementation in src/cache.c. The last line of this function after the for loop is currently uncovered by tests, and is only hit when the entire cache is searched but the object that has been freed is not found.

Next, devise a test for the freeing of an entry structure that is not in the cache. One way to do this is to add a test to test/function/entry.cpp that constructs a new entry normally, allocates a new entry struct dynamically and uses memcpy to copy the normal entry, and then calls the entry destructor on the manually allocated one. Remember that you will need to manually free the dynamic structure memory as well, since the cache_free call will not. Look at the other destructor tests and the create_empty_entry helper function for inspiration for this test.

Make sure that you have full coverage on cache_free after this test. See the test documentation for details on checking coverage.

Finally, refactor cache_free after this to simplify the function to having only one exit pointer. This should be as simple as changing the unlock and return inside the for loop to a break statement. Do this after you have written your test, so that you are sure that the refactored code still works!

twingoof commented 1 week ago

Hello, I'll try to fix this and hopefully come with a PR soon (first time contributing I'll do my best)