views:

152

answers:

8

I see a lot of RAII example classes wrapping around file handles.

I have tried to adapt these examples without luck to a character pointer.

A library that I am using has functions that take the address of a character pointer (declared like get_me_a_string(char **x)). These functions allocate memory for that character pointer and leave it up to the end user of the library to clean it up in their own code.

So, I have code that looks like this...

char* a = NULL;
char* b = NULL;
char* c = NULL;

get_me_a_string(&a);
if(a == NULL){
    return;
}


get_me_a_beer(&b);
if(b == NULL){
    if(a != NULL){
        free(a);
    }
    return;
}


get_me_something(&c);
if(c == NULL){
    if(a != NULL){
        free(a);
    }
    if(b != NULL){
        free(b);
    }
    return;
}

if(a != NULL){
    free(a);
}
if(b != NULL){
    free(b);
}
if(a != NULL){
    free(b);
}

It sounds like RAII is the answer for this mess that I have above. Could someone provide a simple C++ class that wraps a char* rather than a FILE*?

Thanks

+2  A: 

A very basic implementation (that you should make noncopyable etc).

struct CharWrapper {
    char* str;
    CharWrapper(): str() {}  // Initialize NULL
    ~CharWrapper() { free(str); }
    // Conversions to be usable with C functions
    operator char**() { return &str; }
    operator char*() { return str; }
};

This is technically not RAII, as proper initialization happens later than at the constructor, but it will take care of cleanup.

Tronic
I got that far already.I don't know how to actually use it though.How do I declare objects of this type (is it actually an object, you used struct).How do I pass said declared objects to those library functions?
eric.frederich
CharWrapper str1; get_me_a_string(str1); puts(str1); The conversion operators might be somewhat problematic, so consider replacing them with accessor functions. The only difference between struct and class is default visibility. For structs it is public, for classes it is private.
Tronic
I just tested this.Is it supposed to be resistant against segfaults? If so, it doesn't work because the memory isn't free'd. Otherwise it seems to work just fine.The only thing I don't like about this is that in calling printf I now need to cast it as a (char*). Calling other functions seems to work without any cast at all (c++ overloading at work?)
eric.frederich
The type conversion operators allow it to work if the function takes char* or char** argument. Since printf is a vararg function (argument types are not known to the compiler), automatic conversion cannot work.
Tronic
+4  A: 

There's something available already in the standard library: it's called std::string.

Edit: In light of new information:

It will allocate memory and fill it up. I could copy the contents into a new std::string object but I'd still have to free the memory that was allocated by the function.

This is poor design on the implementor's part -- the module that allocates should be responsible for deallocation.

Okay, now that I've got that out of my system: you could use a boost::shared_ptr for freeing.

template<typename T>
struct free_functor
{
    void operator() (T* ptr)
    {
        free(ptr);
        ptr=NULL;            
    }
};
shared_ptr<X> px(&x, free_functor());
dirkgently
I think he's stuck with a library that returns C strings that need to be freed.
Tronic
I don't think `auto_ptr` will work, since it must be a `free()` and not a `delete`. I believe `boost::scoped_ptr` will let you specify a custom deleter, though.
Fred Larson
Actually, I guess `scoped_ptr` doesn't allow a custom deleter. `shared_ptr` does, however.
Fred Larson
I never suggested `auto_ptr` -- if my post gives that feeling, I'd rather edit it. And yes, `shared_ptr` is what I am after. My bad.
dirkgently
@dirkgently: You said it "... may be more trouble than it's worth." In fact, it will work at all if it will only do `delete`. Neither will `boost::scoped_ptr` for the same reason.
Fred Larson
+1. I think `shared_ptr` is a good solution.
Fred Larson
Wait, why did you change to a functor, and what's with the `ptr=NULL`??
Fred Larson
A: 

i think auto_ptr is what you want

or boost shared_ptr if the auto_ptr semantics dont work for you

pm100
auto_ptr deletes the content, but he needs free().
Tronic
ah yes - you can supply customer deleter thoBut I will vote for your answer anyway
pm100
auto_ptr also doesn't play nice with arrays
JohnMcG
A: 

Either use plain std::string, or boost::scoped_array for local arrays, or boost::shared_array for shared strings (the latter allows you to provide custom deleter to call free().)

Nikolai N Fetissov
+1  A: 

You could try something like this:

template <typename T>
class AutoDeleteArray
{
public:
    explicit AutoDeleteArray(const T* ptr)
        : ptr_(ptr)
    {}
    ~AutoDeleteArray()
    {
        delete [] ptr_;
        // if needed use free instead
        // free(ptr_);
    }

private:
    T *ptr_;
};

// and then you can use it like:
{
    char* a = NULL;

    get_me_a_string(&a);
    if(a == NULL)
      return;

    AutoDeleteArray<char> auto_delete_a(a);
}

It is not the most reliable solution, but could be enough for the purpose.

PS: I'm wondering would std::tr1::shared_ptr with custom deleter work as well?

Dmitry
A: 

Thanks everyone for your answers.

Unfortunately, I cannot use boost, or other libraries on this project... so all of those suggestions are useless to me.

I have looked at things like exception handling in C like here... http://www.halfbakery.com/idea/C_20exception_20handling_20macros

And then I looked at why C++ doesn't have a finally like Java does and came across this RAII stuff.

I'm still not sure if I will go the destructor way and make the code C++ only, or stick with C exception macro's (which use the dreaded goto:)

Tronic suggested something like the following. With RAII, or destructors in general, are they supposed to be segfault proof? I'm guessing not.

The only thing that I don't like is the fact that I now have to use a cast (char*) in my printf statements.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct CharWrapper {
    char* str;
    CharWrapper(): str() {}  // Initialize NULL
    ~CharWrapper() {
        printf("%d auto-freed\n", str);
        free(str);
    }
    // Conversions to be usable with C functions
    operator char*()  { return  str; }
    operator char**() { return &str; }
};

// a crappy library function that relies
// on the caller to free the memory
int get_a_str(char **x){
    *x = (char*)malloc(80 * sizeof(char));
    strcpy(*x, "Hello there!");
    printf("%d allocated\n", *x);
    return 0;
}


int main(int argc, char *argv[]){
    CharWrapper cw;
    get_a_str(cw);
    if(argc > 1 && strcmp(argv[1], "segfault") == 0){
        // lets segfault
        int *bad_ptr = NULL;
        bad_ptr[8675309] = 8675309;
    }
    printf("the string is : '%s'\n", (char*)cw);
    return 0;
}
eric.frederich
A: 

An alternative solution would be something like this, which is how I would write this code in C:

char* a = NULL;
char* b = NULL;
char* c = NULL;

get_me_a_string(&a);
if (!a) {
    goto cleanup;
}

get_me_a_beer(&b);
if (!b) {
    goto cleanup;
}

get_me_something(&c);
if (!c) {
    goto cleanup;
}

/* ... */

cleanup:
/* free-ing a NULL pointer will not cause any issues
 * ( see C89-4.10.3.2 or C99-7.20.3.2)
 * but you can include those checks here as well
 * if you are so inclined */
free(a);
free(b);
free(c);
John Ledbetter
In C++ this has the problem that execution might still never reach cleanup because of exceptions. If the code is using exceptions anywhere, you'd also have to throw in a handful of try blocks to make it sure.
UncleBens
Yes, I was debating on doing that (albeit through macros) with this...http://www.halfbakery.com/idea/C_20exception_20handling_20macrosUncleBen: this is actually just plain C code using a C++ compiler. Vistual Studio on Windows and G++ on Linux.
eric.frederich
A: 

Since you are saying you can't use boost, it isn't very hard to write a very simple smart pointer which doesn't share or transfer resources.

Here's something basic. You can specify a deleter functor as a template parameter. I'm not particularly fond of conversion operators, so use the get() method instead.

Add other methods like release() and reset() at will.

#include <cstdio>
#include <cstring>
#include <cstdlib>

struct Free_er
{
    void operator()(char* p) const { free(p); }
};

template <class T, class Deleter>
class UniquePointer
{
    T* ptr;
    UniquePointer(const UniquePointer&);
    UniquePointer& operator=(const UniquePointer&);
public:
    explicit UniquePointer(T* p = 0): ptr(p) {}
    ~UniquePointer() { Deleter()(ptr); }
    T* get() const { return ptr; }
    T** address() { return &ptr; } //it is risky to give out this, but oh well...
};

void stupid_fun(char** s)
{
    *s = static_cast<char*>(std::malloc(100));
}

int main()
{
    UniquePointer<char, Free_er> my_string;
    stupid_fun(my_string.address());
    std::strcpy(my_string.get(), "Hello world");
    std::puts(my_string.get());
}
UncleBens