Saturday, April 10, 2010

If you are using the std::vector or std::deque containers and want to obtain an iterator to a location in your container, don't even think about doing this:

Or this:

So, you can't obtain a valid iterator if you try to obtain it before your container has any items. You also can't obtain a valid iterator if you obtain it while your container has an item, but then you add more items to it and try to use the original iterator afterward.

What gives?

Calls to insert() (or push_back()) when size() == capacity() will force the container to reallocate its memory (they double in size). A new block of memory is allocated and then the old block is copied to the new block, and the old block deleted. This is bad news if you've obtained an iterator to an item in the old block of memory because it still points there, but the data is no longer there.

It should be obvious why calls to reserve() have the same effect.

So, what can you do?

Don't use an iterator obtained prior to a call to insert() or reserve() on your std::vector or std::deque

Keep the iterator updated

Keeping the iterator updated:

Note how the address of our iterator never changes, but the address of the item it points to does change. Therein lies the heart of the matter: When these containers reallocate memory, the data gets moved, but nobody tells the iterators to those pieces of data.

It should also be noted that calls to erase() invalidate any iterators that point to data positioned after the element(s) being erased. The reason should be obvious, as the data is no longer in the same location.

Moral of the story: Don't use an iterator that was obtained prior to calls on your std::vector or std::deque that will cause them to allocate or deallocate memory. Doing so will invalidate your iterators, and using them will yield undefined behavior, which is a nice way of saying "Garbage data at best, An explosion at worst."

For a full explanation of when your iterators will be invalidated (there's more situations than I've listed), consult chapter 23 of the Official C++ Standard (search for all occurrences of the word "valid"):

Monday, March 29, 2010

A common programming interview question is to reverse a string. A common first approach is to copy the contents of the string into a temporary string in reverse order, and then copy the temporary string over the original string. Do this in an interview, and you'll be informed that while your solution is correct, it can be much better. Doing it in this fashion is very inefficient, but it is still nonetheless a good place to start. As always, solve the problem first, then optimize. The naive solution would look something like this:

At this point, the interviewer is going to ask you to perform the reversing "in-place" meaning "do it all in one pass of the string, no double-pass copying shenanigans!"

So, how do we accomplish this? A basic swap should immediately come to mind. You need to swap pairs of characters in the string as you iterate over it until no more pairs exist. But, how to know when you've reached that point? Sure, you could keep track of indices of the string that have been swapped, but at that point you're diverging toward the inefficiency of the first solution because you'd have to do a lookup in the index table each time you attempted a swap, and that gets slower as your string becomes larger. So, storing indices is out. What else can we do? Well, let's see if there is an algorithm behind this.

Given the string "abc", what can we deduct about which pairs should be swapped?

'a' and 'c' get swapped, leaving us with 'b'. Clearly, we see there is nothing else to swap with, but how would we know this internally, from a source code level? The key lies in the positions of the characters in relation to the end of the string. We never want to swap 2 characters unless the location of the second character is > the location of the first character. Using our string "abc", 'a' is located at position 0 and 'c' at position 2. 'b' is located at position 1, but the character at position 2 has already been swapped.

I needed to add a function to determine the intersection (if any) between 2 lines in 2d space. Naturally, already having this utils.h file, I decided to throw it in there. I probably shouldn't have, as it is relatively long and complicated enough that the compiler wasn't going to be able to inline it, but at the time I just threw it in there and tried to do a quick compile:

Huh? That function certainly isn't defined anywhere other than utils.h. So, what gives? My first thought was that I screwed up a header guard somewhere and that coupled with me throwing the definition of the function into the .h file may be causing some issues. So, I checked...

All header guards were fine. Then I started to wonder if it could be a circular dependency of sorts. But, why then is the newly added function the only one causing the linker error? Wouldn't everything else in utils.h also be causing problems? I decide to conduct a test to find out:

I added a very simple function definition to utils.h and placed it above the definition of checkIntersection():

void foo() { int x = 2; }

A quick re-compile, and...

I got the same error as before, except now also with respect to foo(). Looking back over utils.h, it becomes clear that the compiler is inlining the functions declared within my structures. This makes sense, because member functions defined inside of a class definition are implicitly inlined. Inlining means that the functions are replaced by inserting the function bodies at the locations in which they are called. This means the linker never sees them, thus has no reason to complain about them in my situation.

At this point, I have enough information to construct a simple test case to illustrate the issue and better debug it. I need the following setup:

* A utils.h header file that contains a structure which is composed of a trivial data member as well as a function.

* Also in that utils.h file, I need to define a free standing function.

* Lastly, I need to have 2 additional source files, both of which include utils.h, one of which can be the file that holds my main method.

Good. The test case is valid. main.cpp includes both foo.h and utils.h, and foo.h also includes utils.h, thus multiple definitions. Evidently, I'm making a false assumption about what I'm doing, what the header guards are doing, or both... and you know what they say about assuming. Time to stop assuming and starting knowing:

Header guards only prevent multiple definitions within a single translation unit. Take this situation for example:

foo.h - includes utils.h, bar.h
bar.h - includes utils.h

Header guards prevent the contents of utils.h from being included more than once within foo.h (a single translation unit): Once from directly including utils.h, and again when it includes bar.h which in turn includes utils.h.

So, isn't that what I was doing in my example? No, not at all. I was trying to rely on header guards to prevent multiple translation units from defining the same function. After compilation, main.o and foo.o both contain code for utilsFoo() because both main.cpp and foo.cpp include utils.h, which is where utilsFoo() is defined. It isn't being defined more than once per-file, but rather multiple times across all files (translation units). Thus, the linker has multiple object files that all have code for utilsFoo().

So, why does sticking the definition of utilsFoo() into an implementation file as opposed to a header file fix the issue? Because in that case, that implementation file is the only object file that has code for utilsFoo(). When other translation units include utils.h, they are simply making the declaration of utilsFoo() visible to themselves. That is to say, they are making themselves aware that the function exists and can be called, but they aren't pulling in the code for the function.

I feel much better now. Sure, I knew how to fix this from the outset, but it was more important to me to know why I needed to fix it.