views:

276

answers:

6

In the ArduinoUnit unit testing library I have provided a mechanism for giving a TestSuite a name. A user of the library can write the following:

TestSuite suite("my test suite");
// ...
suite.run(); // Suite name is used here

This is the expected usage - the name of the TestSuite is a string literal. However to prevent hard-to-find bugs I feel obliged to cater for different usages, for example:

char* name = (char*) malloc(14);
strcpy(name, "my test suite");
TestSuite suite(name);
free(name);
// ...
suite.run(); // Suite name is used here

As such I have implemented TestSuite like this:

class TestSuite {
public:
  TestSuite(const char* name) {
    name_ = (char*) malloc(strlen(name) + 1);
    strcpy(name_, name);
  }

  ~TestSuite() {
    free(name_);
  }

private:
  char* name_;
};

Putting aside the issue of failing to deal with memory allocation failures in the constructor I'd prefer to simply allocate the pointer to a member variable like this:

class TestSuite {
public:
  TestSuite(const char* name) : name_(name) {
  }

private:
  const char* name_;
};

Is there any way I can change the interface to force it to be used 'correctly' so that I can do away with the dynamic memory allocation?

A: 

Why are you using char* and malloc when you have the nice C++ string class which can takes a string literal or a char* in its constructor ?

Ksempac
Because, unfortunately, avrlibc used by the Arduino (and avr-gcc) doesn't have an STL implementation...
Matthew Murdoch
A: 

Are you able to use std::string? You could have it as std::string name_ and have the STL take care of the memory allocation for you..

class TestSuite {
    public:
      TestSuite(const char* name) : name_(name) {}

      ~TestSuite() {}

    private:
      std::string name_;
};

Don't forget to include <string>.

Reference

DeadHead
No STL is not supported on the Arduino (avr-gcc/avrlibc).
Matthew Murdoch
+1  A: 

Well, you can use a std::string that will take care of all memory allocation

class TestSuite {
public:
  TestSuite(const std::string &name):name_(name) {
  }

  ~TestSuite() {
  }

private:
  std::string name_;
};

Edit : If it is the call to malloc() that you want to avoid you could do this :

class TestSuite {
public:
  TestSuite(const char *name){
    memcpy(name_, name, min(16, strlen(name));
  }

private:
  char name_[16];
};

This will waste some memory however, which can be an issue on embedded platforms.

Ben
I'd like to but avrlibc used by the Arduino (and avr-gcc) doesn't have an STL implementation...
Matthew Murdoch
Oh ok ! Maybe then you should implement something like the stl's string so you don't have to deal with malloc. That is the point of OOP.
Ben
I could do that, but underneath it would still be calling malloc(), wouldn't it so I'm not sure I've really gained anything (except some more code to maintain!).
Matthew Murdoch
Right ! but I don't get your question. You want to be sure that name is a pointer to a valid char* ? Actually what do you mean by "used correctly" ?
Ben
Ok I understood. Sorry.
Ben
+1  A: 

Documentation. For example,

/**
* Test suite constructor.
* @param name test suite name cstring, shared
*/
TestSuite(char const *name) {
// ...

A shared pointer implies that the pointed object must be alive during the lifetime of this object.

laalto
+1 Obviously I've considered this but I'm assuming that, like me, most people don't RTFM! I'm hoping for a code design solution.
Matthew Murdoch
+1  A: 

Have a char name[XYZ] member of your TestSuite (with an XYZ large enough to comfortably display the name) and use strncpy to copy the string (with a maximum length of XYZ-1).

rlerallut
+1 but I'd like not to have to allocate any additional memory if possible. I want to *force* client code to call the library code with a string literal.
Matthew Murdoch
+1  A: 

What if you provide two overloaded constructors?

TestSuite(const char* name) ...
TestSuite(char* name) ...

If called with a const char*, then the constructor could make a copy of the pointer, assuming that the string will not go away. If called with a char*, the constructor could make a copy of the whole string.

Note that it is still possible to subvert this mechanism by passing a const char* to the constructor when the name is in fact dynamically allocated. However, this may be sufficient for your purposes.

I should note that I have never actually seen this technique used in an API, it was just a thought that occurred to me as I was reading your question.

Greg Hewgill
+1 - Great idea. You're right that it could be subverted but it should give better protection against misuse than the original. Obviously if someone wanted to break it they could but that's true of any C/C++. I'll give this a go to see how it works in practice. Thanks.
Matthew Murdoch