Problems with dereferencing a pointer (and returning it)

▲
5
▼

Here I have a function that creates a string, assigns it to a string pointer, and returns it. I tried returning a regular string and it worked fine. But then when when I integrated the pointers and de-referenced them, my program crashed. When I tried to debug it, this is the message I got:

Unhandled exception at 0x00024cbf in Assignment 2.exe: 0xC0000005: Access violation reading location 0xcccccce4.

Here's my code:

string* Recipe::getCookingTime()
// @intput: none
// @output: cooking time as a string
{
    string temp;
    string displayHrs;
    string displayMins;
    if( cookingTime_->numHours < 10 ) 
        displayHrs = intToString(0) + intToString(cookingTime_->numHours );
    else 
        displayHrs = intToString(cookingTime_->numHours );
    if( cookingTime_->numMinutes < 10 ) 
        displayMins = intToString(0) + intToString(cookingTime_->numMinutes);
    else 
        displayMins = intToString(cookingTime_->numMinutes);

    temp = "The time to cook the recipe is " + displayHrs + ":" + displayMins;
    *cTime_ = temp;
    return cTime_;
}
c++
pointers
string
runtime-error
dereference
asked on Stack Overflow Mar 19, 2011 by Oniros • edited Apr 19, 2021 by ouflak

3 Answers

▲
4
▼

The problem is that you are dereferencing the cTime_ variable without actually allocating the memory first. I am not sure if that is a global variable or a member variable but you need to use the "new" operator first to allocate it's memory. So you return the pointer to (address of) this variable back to the caller of the function, but as soon as this function exits, it is deleting the "temp" variable and therefore, the pointer you returned will be pointing at invalid memory.

The solution would be to use the "new" operator:

string* Recipe::getCookingTime()
// @intput: none
// @output: cooking time as a string
{
    string displayHrs;
    string displayMins;
    if( cookingTime_->numHours < 10 ) 
        displayHrs = intToString(0) + intToString(cookingTime_->numHours );
    else 
        displayHrs = intToString(cookingTime_->numHours );
    if( cookingTime_->numMinutes < 10 ) 
        displayMins = intToString(0) + intToString(cookingTime_->numMinutes);
    else 
        displayMins = intToString(cookingTime_->numMinutes);

    if( NULL == cTime_ )
    {
        cTime_ = new string();
    }

    *cTime_ = "The time to cook the recipe is " + displayHrs + ":" + displayMins;
    return cTime_;
}

However, I have to warn you that this is not good design because here you are allocating memory and require that the call knows they have to free it when they are done with it. A preferable way to do it would be to have the caller allocate the variable and then pass in the pointer:

bool Recipe::getCookingTime( string* str )
// @intput: none
// @output: cooking time as a string
{
    if( NULL == str )
    {
        // Received invalid pointer
        return false;
    }
    string displayHrs;
    string displayMins;
    if( cookingTime_->numHours < 10 ) 
        displayHrs = intToString(0) + intToString(cookingTime_->numHours );
    else 
        displayHrs = intToString(cookingTime_->numHours );
    if( cookingTime_->numMinutes < 10 ) 
        displayMins = intToString(0) + intToString(cookingTime_->numMinutes);
    else 
        displayMins = intToString(cookingTime_->numMinutes);

    *str = "The time to cook the recipe is " + displayHrs + ":" + displayMins;
    return true;
}

Then when the caller wants to use the function they can do this:

cTime_ = new string();
getCookingTime( cTime_ );

Summary The important thing to remember here is that you must allocate memory the a pointer is referencing before trying to assign to it. Also, it is generally bad design to allocate memory (using the new operator) within a function and not explicitly delete it. Whoever allocates memory should almost always be the one to free it

answered on Stack Overflow Mar 19, 2011 by drewag • edited Mar 19, 2011 by drewag
▲
2
▼
*cTime_ = temp;

It seems you've not allocated memory for cTime_.

I'm wondering why you're returning pointer to std::string. Why don't you simply return std::string as shown below:

std::string Recipe::getCookingTime()
{
   //your same code
   return temp; //this is fine!
}

Note that the type of return type is changed from std::string* to std::string.

answered on Stack Overflow Mar 19, 2011 by Nawaz
▲
-2
▼

I'll just focus on where your question addresses and not anywhere else in your code. First, I don't think you posted your complete code, because with what you posted, I don't see where cTime_ is defined in that method, so your code won't even compile. Second, assume you define cTime_ as a pointer to a string, and assign that pointer to a the memory occupied by the string temp. When that method exits, temp goes out of scope, now cTime_ no longer points to a valid memory location, thus you get the access violation. You might consider something like this:

void Recipe::getCookingTime( string& str )
{
    string displayHrs;
    string displayMins;
    if( cookingTime_->numHours < 10 ) 
        displayHrs = intToString(0) + intToString(cookingTime_->numHours );
    else 
        displayHrs = intToString(cookingTime_->numHours );
    if( cookingTime_->numMinutes < 10 ) 
        displayMins = intToString(0) + intToString(cookingTime_->numMinutes);
    else 
        displayMins = intToString(cookingTime_->numMinutes);

    str = "The time to cook the recipe is " + displayHrs + ":" + displayMins;
}

Then to call getCookingTime():

string s;
getCookingTime(s);

Instead of dealing with pointer, you would now deal with reference. The code would be more straightforward.

answered on Stack Overflow Mar 19, 2011 by Kevin Le - Khnle

User contributions licensed under CC BY-SA 3.0