Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Problems with dereferencing a pointer (and returning it)

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_;
}
like image 538
Jota Avatar asked Oct 12 '22 12:10

Jota


2 Answers

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

like image 192
drewag Avatar answered Oct 20 '22 12:10

drewag


*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.

like image 37
Nawaz Avatar answered Oct 20 '22 14:10

Nawaz