2 Questions: Memory Leak and Error

[jamesmk]

I'm trying to come up with some code that will bring the substring functionality to a cstring, and I've got two questions. Here is my code.

const char* strsub( const char* cstr, int one, int two ) {
	const char* substr = new char[two+2];
	substr=(cstr+one); 
	cout << *substr << endl;     // Just for testing
	substr[two+1]='\0';          // Line 79
	return substr;
}

My first question is "Is there any way to avoid a memory leak with this function? Ie I believe that the memory used for subsrt will be used until the program ends since I don't destroy[] it. Is that right?"

My second question is "I'm getting the error

review07-2.cpp:79: error: assignment of read-only location ‘*(substr + (((unsigned int)(two - one)) + 1u))’

, but if I don't set *substr to constant, I get an error in the line above. Can someone help me? Should I not be returning a constant?"

Thanks in advance!

[Disch]

if you have a const char* that means you cannot change the data that the pointer points to.

Line 79 changes the data. That's why you get the error. 'substr' should not be a const pointer.

As for the memory leak, you have 3 options:

1) make the calling function responsible for providing the output buffer

ie:
void strsub(char* out,const char* in,int one,int two)

That way, the function does not need to allocate any memory at all. This is what is done by strcpy and similar standard lib functions.

2) make the calling function responsible for deleting the allocated memory.

ie:

char* foo = strsub("stuff",1,3);
cout << foo;
delete[] foo;  // since I called strsub, I'm responsible for cleanup 

3) use strings instead of char arrays.



Obviously #3 is the best. But if you can't use strings for whatever reason #1 would be my next recommendation. I really don't recommend #2 at all, but I mentioned it for completeness.


You also have another big problem here:

char* substr = new char[two+2];  // allocates memory, substr points to allocated memory
//substr=(cstr+one);  // BAD.  this does not copy the string.
strcpy(substr,cstr+one); // you probably meant to do this 

note that when you do substr=(cstr+one);, substr no longer points to the memory you allocated. Instead it points to the same buffer that cstr points to.

[jamesmk]

Thanks for the help, Disch.

I ended up going another way, but it still has the memory-leak because I'm still returning a pointer to an array.

const char* strsub( const char* cstr, int one, int two ) {
	char* substr = new char[two+2];
	int i;
	for (i=0; i < two ; i++)
		substr[i]=cstr[one+i];
	substr[++i]='\0';
	return substr;
}

I'll pout about it for a while, then probably go with your solution #1.

thx