Saturday 12 February 2011

Friends Don’t Let Friends Use Varargs

Whilst writing my previous post about being clever with strings and varargs functions I was reminded of another time that I had a run-in with them. Luckily I wasn’t the protagonist this time but a colleague instead. He decided that he wanted to make it easier to add an arbitrary set of strings to a particular class via the ctor. In modern day C++ you might do it via an iterator pair like this:-

{
  const char* words[] = { “cat”, “mat”, “dog” }

  Thingy thingy(words, words + ARRAYSIZE(words));
  . . .
}

However this was many years ago and the project was MFC based so the use of STL idioms were very thin on the ground[$].

Another less obvious choice might be to define a vector, add them to that and define a ctor that takes a vector of strings. But nope, my colleague decided that he could avoid the ugly array definition and performance hit of the vector approach by declaring a varargs ctor like so:-

class Thingy
{
public:
  Thingy(const char* word1, ...);
};

This meant that you could provide an arbitrary parameter list of strings and then terminate it with a NULL parameter:-

{
  Thingy things(“cat”, “mat”, “dog”, NULL);
  . . .
}

Naturally I suggested you don’t mess with varargs and that this was an accident waiting to happen[#]. I did agree that it was not an uncommon technique to define an array of items and use a NULL terminator at the end and that what he was effectively doing was this:-

{
  const char* words[] = { “cat”, “mat”, “dog”, NULL }

  Thingy thingy(words);
  . . .
}

At the time we had a definite need to support up to three arguments which were actually all CStrings. I’m sure I suggested writing three ctor overloads that then call a common internal helper method instead for now:-

class Thingy
{
public:
  Thingy(CString arg1);
  Thingy(CString arg1, CString arg2);
  Thingy(const CString& arg1, . . . arg2, . . . arg3);
};

But no, that was far too much work, his way was simpler and, after all, we were all experienced programmers so we don’t make those kind of silly mistakes…

Sadly I can’t remember when the first bug showed up or why it remained hidden for so long[+], but one did suddenly appear and guess what was missing? Yup, the NULL terminator. Of course now I had to check for any other occurrences of malformed ctor calls and I soon discovered one more. Clearly someone was not aware of this need to add a terminator so I did a little Software Archaeology and rooted around in our VCS to find out who needed a little educating… But of course, it was my esteemed colleague, the very same person who suggested that we wouldn’t make that kind of mistake!

So remember kids – Just Say No to Vargs Functions. Unless of course you’re doing Template Meta-Programming in which case you’ll be [ab]using them for a different reason.

 

[$] Due to it’s age MFC has its own containers and single iterator type and doesn’t adhere to the STL idioms that all all know and love now. Consequently if you spent your years knee deep in MFC code you could very easily to be unaware of them.

[#] Of course this was many years before I became aware of my own misfortunes with varargs function and anyway I’d read all the books by Scott Meyers, Herb Sutter, et al so that made me the fountain of all knowledge, right?

[+] It just goes to show that even with debugging techniques like the compiler using guard blocks and filling uninitialized stack space with a non-zero pattern it’s still possible for the stack to end up correctly formed by luck and not by judgement. That’s especially true when the stack variable is a ‘bool’.

No comments:

Post a Comment