Background

• The outer-most list represents “rows”. It collects all objects found. It has zero or more entries.
• The inner-most list represents a single field. It is a key value pair. It is a list of size 2. The first entry is the name of the field (column to FitNesse). The second field is the value. Both are strings.
• The middle list represents a single object. It is a collection of fields. It has zero or more entires.

I understand the need for this representation and it takes a little bit to get it built correctly. So much so, I built a simple tool to do it in java.

C++ is no different. In fact, the authors of cslim though the same thing and they created a C Abstract Data Type to help out (and an add-on method to create the correct final form):

• SlimList
• SlimList_Serializer

They use C because it can be used by both C and C++. I’m using C++ and I wanted to make building query results even easier, so I built a QueryResultAccumulator. The most recent source is in the previous blog and I’ll be putting it on github after I’ve had some more time to work on it.

Here’s the progression to my QueryResultAccumulator class:

• Wrote a Query-Table based fixture and followed the example provided with cslim (thank you for that!)
• Moved the code from functions into methods on a class
• Extracted a class, called SlimListWrapper, which made the fixture code easier to follow.
• Went to get takeout and realized that I had named the class incorrectly and that it was really accumulating query results (thus the name). The SlimList was a mechanism, not an intent.
• Refactored the class into QueryResultAccumulator (I left the original alone, created a new class, copied code from one to the other and changed it around a bit.

Now it might sound like I didn’t have any tests. In fact, I did. I had my original Acceptance Test in FitNesse. I kept running that, so in a sense I was practicing ATDD.

I was not happy with that, because I was not sure that I had properly handled memory allocation correctly. In fact, I had not. The final result is dynamically allocated and I was not releasing that. So I “fixed” it. (It needs to be released after the return from slim back to FitNesse, so the typical pattern is to lazily delete it, or release it in a “destroy” method called after the execution of a single table.)

I have a memory leak?!

TEST(QueryResultAccumulator, CanProduceFinalResultsMultipleTimes) {
    QueryResultAccumulator accumulator;
    accumulator.produceFinalResults();
    accumulator.produceFinalResults();
}

This caused a memory leak of 60 bytes. It was at this point I was up too late and banging my head against a wall. About 3 hours later I figured that out and went to bed. I fixed the problem and sent a patch to authors of cslim in maybe 45 minutes. So I should have gotten more sleep.

Where is that damn thing

  if(result)
    free(result);

This was the correct thing, but it was in the wrong location. Again, this was related to a preprocessor seam in cslim.

Eureka!

cslim depends on CppUTest and uses a different malloc/free

Ah ha! That’s it. So I tired to recompile my C++ code to use the same thing. However, I was not able to do that. CppUTest’s memory tracking implementation does not work with many of the C++ standard classes like <string> and <vector>. So I could not compile my code using the same approach.

I’m glad this happened becuase it made me realize that it was the wrong place anyway. Here’s the logic:

• CSlim has a preprocessor seam, comile with or without -Dfree=cpputest_free, -Dmalloc=cpputest_malloc
• I’m using a class in cslim to do the allocation, where the policy of allocation is stored
• I should not release the memory but instead allow the cslim library to release the memory becaue it has the allocation policy, and therefore the release policy.

cslim/include/CSlim/SlimListSerializer.h:

void SlimList_Release(char *serializedResults);

cslim/src/CSlim/SlimListSerializer.c:

void SlimList_Release(char *serializedResults)
{
  if(serializedResults)
    free(serializedResults);
}

I updated my QueryResultAccumulator to use SlimList_Release and my false positive disappeared.

It also turns out that this improved symmetry in the library. To allocate and release entries in an SlimList you use the following functions:

• SlimList* SlimList_Create()
• void SlimList_Destroy(SlimList*);

Now to serlalize a list and release the memory later you use:

• SlimList_Serialize
• SlimList_Release

As I write this, I think there’s a better name. I’ll let the authors give it a better name (like SlimList_Release_Serialization_Results). But in any case, if you use a function in the cslim library that allocates something, you use another method in the cslim library to release it.

Since the libray has a preprocessor seam, that symmetry removes a false-positive memory leak.

What took so long?

When I called free in the unit test, it was calling the correct version of free, cpputest_free, becasue it was a unit test using CppUTest. When I moved the code into the class, which has no knowledge of CppUTest (nor can it), the flow of the code was correct, but the compilation (preprocessor symbols) were different and it caused a false positive.

Since I changed the code, I assumed it was a problem with how I changed the code. To me more clear, I though it was a code-flow problem, not a preprocessor seam problem. So I spent a lot of time verifyig my code. Once I determined it was correct, I then moved into debugger mode.

It was not long after that when I finally figured out what was going on.

Conslusions

That cslim depends on CppUTest might be questionable. However, if I treat cslim as a (mostly) black box, and I don’t know its allocation policy, I should not assume a deallocation policy.

By putting the responsiblity in the correct library level, it fixed the problem and added symmetry to the overall solution.

I also really enjoyed this (after it was done). I’ve come across memory leaks using CppUTest in the past. Often they were my fault. Sometimes they were not. This was interesting because it both was and was not my fault. I originally had written the code incorrectly. When I put he correct steps in my code, I still had wrong code because I put the responsibility in the wrong place. It really was only correct after I moved the actual implementation into the library and then called it from my code that I had finally written it correctly.