8

I am working in C++ with two large pieces of code, one done in "C style" and one in "C++ style".

The C-type code has functions that return const char* and the C++ code has in numerous places things like

const char* somecstylefunction();
...
std::string imacppstring = somecstylefunction();

where it is constructing the string from a const char* returned by the C style code.

This worked until the C style code changed and started returning NULL pointers sometimes. This of course causes seg faults.

There is a lot of code around and so I would like to most parsimonious way fix to this problem. The expected behavior is that imacppstring would be the empty string in this case. Is there a nice, slick solution to this?

Update

The const char* returned by these functions are always pointers to static strings. They were used mostly to pass informative messages (destined for logging most likely) about any unexpected behavior in the function. It was decided that having these return NULL on "nothing to report" was nice, because then you could use the return value as a conditional, i.e.

if (somecstylefunction()) do_something;

whereas before the functions returned the static string "";

Whether this was a good idea, I'm not going to touch this code and it's not up to me anyway.

What I wanted to avoid was tracking down every string initialization to add a wrapper function.

pythonic metaphor
  • 10,296
  • 18
  • 68
  • 110
  • I would make mention that you're "maintainer," as you referred to him in a comment, might be breaking some pretty important style rules. I don't know why you'd want to return a pointer to a char unless it is being passed INTO the function via pointer. Could you elaborate on this? You might have bigger problems. – San Jacinto Nov 12 '09 at 18:13
  • 1
    Can anyone answer why the std::basic_string implementation doesn't treat a null value_type pointer as an empty string? – jmucchiello Nov 12 '09 at 18:14
  • @jmucchiello, I think because fundamentally, an "empty string" is very different from a pointer which points to memory address 0. An empty string `""` actually takes up memory as a single null char. – Charles Salvia Nov 12 '09 at 18:18
  • 1
    Have you tried talking to whoever made the decision to start returning null pointers, and explaining that it's breaking the C++ code? – David Thornley Nov 12 '09 at 18:54
  • 1
    If the functions returned "", it should still be possible to do easy checks: `if (*somecstylefunction())` – UncleBens Nov 12 '09 at 19:57
  • @uncle that's no the point... you have to go back and update the code. – San Jacinto Nov 13 '09 at 03:37

7 Answers7

14

Probably the best thing to do is to fix the C library functions to their pre-breaking change behavior. but maybe you don't have control over that library.

The second thing to consider is to change all the instances where you're depending on the C lib functions returning an empty string to use a wrapper function that'll 'fix up' the NULL pointers:

const char* nullToEmpty( char const* s)
{
    return (s ? s : "");
}

So now

std::string imacppstring = somecstylefunction();

might look like:

std::string imacppstring( nullToEmpty( somecstylefunction());

If that's unacceptable (it might be a lot of busy work, but it should be a one-time mechanical change), you could implement a 'parallel' library that has the same names as the C lib you're currently using, with those functions simply calling the original C lib functions and fixing the NULL pointers as appropriate. You'd need to play some tricky games with headers, the linker, and/or C++ namespaces to get this to work, and this has a huge potential for causing confusion down the road, so I'd think hard before going down that road.

But something like the following might get you started:

// .h file for a C++ wrapper for the C Lib
namespace clib_fixer {
    const char* somecstylefunction();
}


// .cpp file for a C++ wrapper for the C Lib
namespace clib_fixer {
    const char* somecstylefunction() {
        const char* p = ::somecstylefunction();

        return (p ? p : "");
    }
}

Now you just have to add that header to the .cpp files that are currently calling calling the C lib functions (and probably remove the header for the C lib) and add a

using namespace clib_fixer;

to the .cpp file using those functions.

That might not be too bad. Maybe.

Michael Burr
  • 333,147
  • 50
  • 533
  • 760
  • "and probably remove the header for the C lib" - I'm pretty sure you do have to remove it. I think otherwise the call to `somecstylefunction` will be ambiguous between `namespace clib_fixer` and `namespace ::`. – Steve Jessop Nov 12 '09 at 23:15
6

Well, without changing every place where a C++ std::string is initialized directly from a C function call (to add the null-pointer check), the only solution would be to prohibit your C functions from returning null pointers.

In GCC compiler, you can use a compiler extension "Conditionals with Omitted Operands" to create a wrapper macro for your C function

#define somecstylefunction() (somecstylefunction() ? : "")

but in general case I would advise against that.

AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765
  • yeah, it seems like something I'd want to steer clear of. It's probably better to fix the source of the problem than the symptoms. – San Jacinto Nov 12 '09 at 18:07
  • Some form of macro or a set of your own wrappers around the library would be a good way to solve the problem without changing the original library. – Anton Nov 12 '09 at 18:11
  • 1
    @antonmarkov: Yes, but the combination of macro trickery *and* compiler extension is a bit too much in my opinion. – AnT stands with Russia Nov 12 '09 at 18:12
3

I suppose you could just add a wrapper function which tests for NULL, and returns an empty std::string. But more importantly, why are your C functions now returning NULL? What does a NULL pointer indicate? If it indicates a serious error, you might want your wrapper function to throw an exception.

Or to be safe, you could just check for NULL, handle the NULL case, and only then construct an std::string.

const char* s = somecstylefunction();
if (!s) explode();
std::string str(s);
Charles Salvia
  • 52,325
  • 13
  • 128
  • 140
  • In this case, it does not indicate error. The maintainer decided that it was cleaner for "no message" to be indicated with a null pointer rather than a pointer to the static "". Which isn't unreasonable, but breaks this other code. – pythonic metaphor Nov 12 '09 at 18:06
  • In that case, you could simply use an inline wrapper function which tests for NULL, and returns an empty std::string if the pointer is NULL. – Charles Salvia Nov 12 '09 at 18:08
2

For a portable solution:

(a) define your own string type. The biggest part is a search and replace over the entire project - that can be simple if it's always std::string, or big one-time pain. (I'd make the sole requriement that it's Liskov-substitutable for a std::string, but also constructs an empty string from an null char *.

The easiest implementation is inheriting publicly from std::string. Even though that's frowned upon (for understandable reasons), it would be ok in this case, and also help with 3rd party libraries expecting a std::string, as well as debug tools. Alternatively, aggegate and forward - yuck.

(b) #define std::string to be your own string type. Risky, not recommended. I wouldn't do it unless I knew the codebases involved very well and saves you tons of work (and I'd add some disclaimers to protect the remains of my reputation ;))

(c) I've worked around a few such cases by re-#define'ing the offensive type to some utility class only for the purpose of the include (so the #define is much more limited in scope). However, I have no idea how to do that for a char *.

(d) Write an import wrapper. If the C library headers have a rather regular layout, and/or you know someone who has some experience parsing C++ code, you might be able to generate a "wrapper header".

(e) ask the library owner to make the "Null string" value configurable at least at compile time. (An acceptable request since switching to 0 can break compatibility as well in other scenarios) You might even offer to submit the change yourself if that's less work for you!

peterchen
  • 40,917
  • 20
  • 104
  • 186
2

You could wrap all your calls to C-stlye functions in something like this...

std::string makeCppString(const char* cStr)
{
    return cStr ? std::string(cStr) : std::string("");
}

Then wherever you have:

std::string imacppstring = somecstylefunction(); 

replace it with:

std::string imacppstring = makeCppString( somecystylefunction() );

Of course, this assumes that constructing an empty string is acceptable behavior when your function returns NULL.

underscore_d
  • 6,309
  • 3
  • 38
  • 64
JohnMcG
  • 8,709
  • 6
  • 42
  • 49
  • `std::string imacppstring = makeCppString(somecystylefunction);` will not work because you're passing the address of the function, not its returned `char const *`. – underscore_d Jul 02 '16 at 16:50
1

I don't generally advocate subclassing standard containers, but in this case it might work.

class mystring : public std::string
{
    // ... appropriate constructors are an exercise left to the reader
    mystring & operator=(const char * right)
    {
        if (right == NULL)
        {
            clear();
        }
        else
        {
            std::string::operator=(right);  // I think this works, didn't check it...
        }
        return *this;
    }
};
Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
  • @Walter, I never said it was a *good* idea, just a way to make the best of a bad situation. And it works: http://ideone.com/2xGd1A – Mark Ransom Feb 26 '14 at 20:38
  • With C++11, in order for move semantics to work between `std::string` and `mystring` (in either direction), which other members (apart from the constructors) do I need to overload? Don't I need an rvalue conversion to `std::string&&` ? – Walter Feb 28 '14 at 08:42
0

Something like this should fix your problem.

const char *cString;
std::string imacppstring;

cString = somecstylefunction();
if (cString == NULL) {
  imacppstring = "";
} else {
  imacppstring = cString;
}

If you want, you could stick the error checking logic in its own function. You'd have to put this code block in fewer places, then.

Carl Norum
  • 219,201
  • 40
  • 422
  • 469