Issues with dereferencing a pointer (and returning it)

Question

here I have a function that creates a string, assigns it to a pointer to a string, and returns it. I tried to return the regular line, and everything turned out fine, but then, when I turned on the pointers and references to them, my program crashed. When I tried to debug it, this is the message I received:

Unhandled exception at 0x00024cbf in Assignment 2.exe: 0xC0000005: read access violation location 0xcccccce4

Here is 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_; } 

Hope someone can help me, thanks!

Answer 1

The problem is that you are casting the cTime_ variable without first allocating memory. I'm not sure if this is a global variable or a member variable, but first you need to use the “new” operator to highlight it. This way, you return a pointer to the (address) of this variable back to the calling function, but as soon as this function completes, it deletes the "temp" variable, and therefore the pointer you return will point to 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 must warn you that this is not a very good design, because here you allocate memory and require the call to know that it should free it when it ends with it. The preferred way to do this should be for the caller to allocate the variable and then pass 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, he can do this:

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

Summary It is important to remember that you must allocate the memory that the pointer points to before attempting to assign it. In addition, the generally poor design allows you to allocate memory (using the new operator) inside the function and not explicitly delete it. The one who allocates memory should always always free it

Answer 2

 *cTime_ = temp; 

It seems you have not allocated memory for cTime_ .

I am wondering why you are returning a pointer to std::string . Why don't you just return std::string , as shown below:

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

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

Answer 3

I will just focus on where your questions are being addressed, not anywhere else in your code. Firstly, I don’t think you posted your full code, because with what you posted, I don’t see where cTime_ is defined in this method, so your code will not even compile. Secondly, suppose you define cTime_ as a pointer to a string and assign this pointer to the memory occupied by the temp string. When this method goes out, temp goes out of scope, now cTime_ no longer indicates the actual place in memory, thus you get an access violation. You might think 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 a pointer, you are now dealing with a link. The code would be simpler.