10

Below code expected to print "kevin" But, it's printing garbage value. I have checked in debugger. The pointer returned by "operator char*" call is invalid. Any idea?

class Wrapper
{
private:
    char* _data;

public:

    Wrapper(const char* input)
    {
        int length = strlen(input) + 1;
        _data = new char[length];
        strcpy_s(_data, length, input);
    }

    ~Wrapper()
    {
        delete[] _data;
    }

    operator char*()
    {
        return _data;
    }
};

int main()
{
    char* username = Wrapper("kevin");
    printf(username);
    return 0;
}
4
  • 6
    My first suggestion would be to just use std::string since you're obviously using C++. Aug 11, 2010 at 19:13
  • why did you edit out the pointers tag i added?
    – James
    Aug 11, 2010 at 19:14
  • Do keep in mind you should generally avoid implicit conversions. (And of course, use std::vector, or std::string, etc.)
    – GManNickG
    Aug 11, 2010 at 19:34
  • Tedious historical detail: prior to the ANSI standard, C++ didn't specify when temporaries should destruct. The last time I used Sun's C++ compiler it still defaulted to destructing them at the end of the enclosing block, so the above program would work. Aug 12, 2010 at 8:18

4 Answers 4

16

The problem is that your Wrapper object is being constructed as a temporary and immediately destroyed. Through the operator char* you're returning a pointer to memory that has been deleted by the Wrapper object when it was destroyed.

To make it work:

Wrapper wrapper("Kevin");
char* username = wrapper;
2
  • 2
    @sankaran1984 Do him/her a favor and accept the answer, its the big checkmark to the left. =P
    – James
    Aug 11, 2010 at 19:20
  • 1
    Might be nice for answerers to upvote a well-expressed question from a SO newbie too.
    – anon
    Aug 11, 2010 at 19:25
5

This line:

char* username = Wrapper("kevin");

creates a nameless Wrapper object which is immediately destroyed, leaving your pointer pointing at nothing. You need to either give the wrapper object a name, or not write code like that. This would work:

Wrapper w("kevin");
char* username = w;
printf( "%s", username );
4

The pointer you are returning is being deleted by the Wrapper destructor as soon as it goes out of scope at the end of the first statement - before the printf.
In some cases, this bug can be covered up because the memory may not be reclaimed immediately and the value "looks" OK even though it is no longer valid. Some tools can help detect this.

1

This statement creates a temporary Wrapper object:

char* username = Wrapper("kevin");

At the end of the statement, ("full expression") the Wrapper object gets destroyed. You're left with a dangling pointer (i.e., what it points at has been deleted).

Returning a pointer (or reference) to an object's internal data is generally dangerous and should be avoided when at all reasonable. In any case, don't you think there are enough string classes in the world? Do you really need to write another?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.