Class + Memory issues

I have this class, with 3 char-arrays that are initialized like this:

Code:

---

this->name = (char*) malloc(64);
this->type = (char*) malloc(64);
this->item_class = (char*) malloc(64);

---

After that, I have a constructor, that copies values to these from a file (read until \n), which ALWAYS work perfectly. I am 100% sure this function is not wrong, since it has worked before, and I haven't changed it. This function copies everything to its correct place.

Now. My problem is:

I have a function that enchants a weapon (warhammer in this case), and because of that, appends the enchantments name to the weapons name (this->name).

warhammer <enchantments name>

These enchantment names are less than 20 characters long. And the weapon names are less than 20 characters long aswell.

Example:

I have a warhammer that I enchant, and when it gets some enchants (not all, some enchants work fine) like " of minor striking" or " of minor intellect", somehow my this->item_class gets overwritten and now says ing or lect.
This shouldnt happen, since I have already allocated memory for up to 64 chars.
The only thing the enchant function does, is appends the enchantments name to this->name.
this->item_class remains untouched.

Why does this happen?

I make it a rule to allocate ATLEAST 64 bytes of memory for all strings, and I have nothing in my program that takes up more than 15-20 bytes on its own.
Add enchant and weapon name together and you still only get at most a 40-byte string.

P.S. this->item_class contains a small string, such as exotic, two-handed or ranged.
I use strcat to append the enchantments name.

why dont you use C++ style strings, and stringstreams to do this?
it would be simple then you would not have to manage the memory.

It is preferred that std::strings be used in place of raw char * they are much easier to work with and such. But if you want to do things the old way you could at least use new C++ memory allocation(yes it was a pun). Basically instead of calling malloc and free you use new and delete. Example:

Code:

---

this->name = new char[64];
//do stuff with the name
delete [] this->name;

---

It is a lot cleaner because you don't have to cast the void* that malloc returns and it is more conventional C++ practices.
As a side note if you allocate with new[], deallocate with delete[] and if you use new, deallocate with delete.

Is it bad to use new to allocate, and delete[] to deallocate? or can you do that and still be safe. I have always used delete[] no matter what, unless it was to delete class instances. although I am obviously not the expert here.

Quote:

---

Originally Posted by Siphon

After that, I have a constructor, that copies values to these from a file (read until \n), which ALWAYS work perfectly. I am 100% sure this function is not wrong, since it has worked before, and I haven't changed it.

---

These sorts of assumptions can be the cause of weeks of wasted time. You never look where the problem is because "It can't possibly be there." Well, maybe it isn't, but the fact that a function has worked in the past is proof of nothing. It could have been working by accident.

So you say the buffers can't be overflowing. Yet clearly they are (characters from the name are leaking into some other buffer). Without seeing more code there is no way to tell why this happens.

Quote:

---

Originally Posted by prog-bman

It is a lot cleaner because you don't have to cast the void* that malloc returns and it is more conventional C++ practices.

---

Uh, you don't have to cast the return value of malloc(). Or any void *, for that matter. If the compiler complains when you do not cast, it means you forgot to include stdlib.h

Yes the operators are meant to be paired together so allocate with new, deallocate with delete, allocate with new[], deallocate with delete[].

Quote:

---

Originally Posted by brewbuck

Uh, you don't have to cast the return value of malloc(). Or any void *, for that matter. If the compiler complains when you do not cast, it means you forgot to include stdlib.h

---

If you are talking about C then you are correct. But in C++ you need to cast the void to something because of the increased type saftey in C++.
I don't have a GCC compiler here at work but I do have Visual C++ 2005 which says to me this when I do this

Code:

---

        void *ptr = new int[10];
       
        int *intPtr = ptr;

---

Error 1 error C2440: 'initializing' : cannot convert from 'void *' to 'int *' c:\documents and settings\bjohn\my documents\visual studio 2005\projects\temp\temp\temp.cpp 11

Now if I cast it with static_cast<> it loves me agian.

Code:

---

        void *ptr = new int[10];
       
        int *intPtr = static_cast<int*>(ptr);

---

Output from GCC:

Code:

---

void.cpp:2: error: invalid conversion from `void*' to `int*'

---

You can't use static_cast<> to cast between pointer types, you have to use reinterpret_cast<>.

>> You can't use static_cast<> to cast between pointer types, you have to use reinterpret_cast<>.
Sorry Noir, that's not true. reinterpret_cast is used to cast from pointer to integral type and back, but a static_cast is fine in many cases for casting from pointer to pointer.

Yeah, you're right. I was thinking of something else.