Memory Allocation In A String Class

I'm trying to make my own string class. I wrote a member function to append to a string, but I'm not sure if its very good.

First off at the start of the code I make a pointer that points to the existing string. Then I make a new string, of the combined size. Then I copy the contents of the original, then the string to add to the new string.

I got 2 questions here. First is: Will the memory stored at 'origin' be freed when it goes out of scope?

Also instead of making a new string of the combined size, is it possible to resize the amount of space allocated to 'str_ptr' whilst retianing its original slot in memory? That way I wouldent have to copy its existing contents over. Or could that cause problems with whatevers stored next to it? Anyway heres my function:

Code:

---

void String::Add(char *add)
{
        char *origin = str_ptr;
       
        //Get New Size And Resize String
        char *tmp = add;
        while(*tmp)size++, tmp++;
        str_ptr = new char[size];
       
        char *ptr = str_ptr;
        while(*origin) *ptr=*origin, origin++, ptr++; //Copy original string
        while(*add) *ptr = *add, ptr++, add++;                  //Copy string to append
        ++*ptr='\0';
}

---

str_ptr and size are class variables.

Quote:

---

Will the memory stored at 'origin' be freed when it goes out of scope?

---

No, you have to delete[] origin.

Quote:

---

Also instead of making a new string of the combined size, is it possible to resize the amount of space allocated to 'str_ptr' whilst retianing its original slot in memory? That way I wouldent have to copy its existing contents over. Or could that cause problems with whatevers stored next to it?

---

I am still not entirely sure about this, but C++ does not provide an equivalent for realloc() along the lines of new[], so the simple answer is that it is not possible.

Ok, thanks for explaining that. I tried using delete but now I get more errors. Basically I tried putting: delete origin;, delete []origin;, and delete[] origin; at the end of my function but in each case when I run the progam I get a dialog appear saying this:

Debug Assertion Falied

expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)
And some other stuff. I think it might have something to do with that the memory was originally assigned to the string object, but I'm not sure. Any Ideas on how to fix this?

Quote:

---

Originally Posted by mike_g

Ok, thanks for explaining that. I tried using delete but now I get more errors. Basically I tried putting: delete origin;, delete []origin;, and delete[] origin; at the end of my function but in each case when I run the progam I get a dialog appear saying this:

Debug Assertion Falied

expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)
And some other stuff. I think it might have something to do with that the memory was originally assigned to the string object, but I'm not sure. Any Ideas on how to fix this?

---

Sounds like you are writing past the end of a buffer - or you're freeing the wrong stuff... Make sure you use delete[] and not delete <without square brackets>.

--
Mats

Well given that the code you have shows a memory overwrite, I guess there are other memory overwrites in your other code.
Your size doesn't count the space for the \0

> expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)
In addition to the block of memory you see, the environment also maintains a header block (usually just before your block) of information which describes the block allocated to you (it's size for example).
What this assert is telling you is that at the point you try to delete the block, it has found that the associated header block has been corrupted.
You need to find out which other allocation is going way beyond the normal boundaries of where it should go.

> while(*tmp)size++, tmp++;
This is a very poor use of the comma operator to avoid using braces.

Quote:

---

Originally Posted by Salem

> while(*tmp)size++, tmp++;
This is a very poor use of the comma operator to avoid using braces.

---

Not to mentioon that size = strlen(add) does the same thing, is most likely faster, and doesn't require any extra "tmp" variable [in this function at least].

--
Mats

Quote:

---

Well given that the code you have shows a memory overwrite, I guess there are other memory overwrites in your other code.
Your size doesn't count the space for the \0

---

The existing size should account for the null char, and since you only need one in the string I don't think thats a problem here. But I'll go over it just to be sure. Also, I'm not writing anything to 'origin', just reading from it. And it stops when it reaches the null char, which already exists.

Quote:

---

> expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)
In addition to the block of memory you see, the environment also maintains a header block (usually just before your block) of information which describes the block allocated to you (it's size for example).
What this assert is telling you is that at the point you try to delete the block, it has found that the associated header block has been corrupted.
You need to find out which other allocation is going way beyond the normal boundaries of where it should go.

---

Could that be a result from doing this?:

Code:

---

char *origin = str_ptr;

---

Quote:

---

> while(*tmp)size++, tmp++;
This is a very poor use of the comma operator to avoid using braces.

---

would this be better?:

Code:

---

while(*tmp) {size++; tmp++;}

---

Or is it just that beacuse its all on one line? Tbh, I don't really know any other uses for the comma operator yet.

Quote:

---

Not to mentioon that size = strlen(add) does the same thing, is most likely faster, and doesn't require any extra "tmp" variable [in this function at least].

---

Yeah I tried including <string.h>, but I got a warning saying its depricated, so I just made my own strlen code.

Quote:

---

Originally Posted by mike_g

Yeah I tried including <string.h>, but I got a warning saying its depricated, so I just made my own strlen code.

---

try using <cstring> instead - that shoudl be OK.

--
Mats

Ok, I'll do that then :)

Quote:

---

Originally Posted by mike_g

Code:

---

while(*tmp) {size++; tmp++;}

---

Or is it just that beacuse its all on one line? Tbh, I don't really know any other uses for the comma operator yet.

---

It's better, but it's probably even better to write it over 4 lines or whatever it takes. Or, if you want it nice and succinct:

Code:

---

for(size = 0; add[size]; size++) ;

---

Most people won't understand it, but it's nice and short, doesn't use any unnecessary variables, or such.

--
Mats

Yeah, personally I like to do little things like that all on one line then maybe add a comment saying what it does, but I guess it may not be as readable for everyone else.

Quote:

---

for(size = 0; add[size]; size++) ;

---

I hadent though about doing it in a for loop, but I dont think it would work for my code as size is already set to the size of the existing string, and then i'd have to find the string length of 'add' first.

Quote:

---

Originally Posted by mike_g

Yeah, personally I like to do little things like that all on one line then maybe add a comment saying what it does, but I guess it may not be as readable for everyone else.

I hadent though about doing it in a for loop, but I dont think it would work for my code as size is already set to the size of the existing string, and then i'd have to find the string length of 'add' first.

---

Ah, sorry, didn't look too carefully at the code. You would have to introduce a new variable then, e.g.

Code:

---

for(int i = 0; add[i]; i++, size++) ;

---

I still think size += strlen(add) is better tho'.

--
Mats

Yeah, I'll use strlen. Also I think I found whats causing this problem:

expression: _BLOCK_TYPE_IS_VALID(pHead->nBlockUse)

Its because I was incrementing origin, so it was no longer at its start position. Thanks for the help guys.