VC6 debug error

str1 and str2 point to the same memory location and you're deleting it twice.

This assert error is caused by the "delete [] str2" because the memory pointed by str2 was previously deleted in "delete [] str1"

[edited by - Hedos on April 25, 2004 9:26:19 PM]

quote:

---

Original post by Anonymous Poster
String 2 has the address of string one. And so basically, you delete string 1, then you delete the memory that string 2 points to, which was already freed.

SO the debug error is telling you that you are freeing memory already freed memory.

---

ok that sounds correct. It is clear that str2 still holds the
memory address of the data that str1 used, and the program is
attempting to delete the already freed memory.

Is there some way of keeping track of strings ( or pointers
in general ) of this sort?
I would use strcpy(...) but it cannot copy strings created with
the new keyword.

I would greatly appreciate some sort of solution to this
potential bug.

Fixed!!!

The source below compiles and runs fine. The solution is
to use a pointer to a pointer when more than one pointer
to the same data is required!

// Console6.cpp : Defines the entry point for the console application.

//


#include "stdafx.h"
#include <iostream.h>


char* str1;
char** str2;

int main(int argc, char* argv[])
{
	str1 = new char[ 7 ];
	str1[0] = ''a'';
	str1[1] = '' '';
	str1[2] = ''w'';
	str1[3] = ''o'';
	str1[4] = ''r'';
	str1[5] = ''d'';
	str1[6] = NULL;

	str2 = &str1

	cout<< str1 << endl;
	cout<< *str2 << endl;

	if( str1 )
	{
		delete [] str1;
		str1 = NULL;
	}
	if( str2 ) 
	{
		delete [] *str2;
		str2 = NULL;
	}

	return 0;
}

quote:

---

Original post by ray_intellect
I would use strcpy(...) but it cannot copy strings created with
the new keyword.

---

Why strcpy() couldn't do that?

char *string;
string = new char[64];

strcpy(string,"Hello!");
cout << string << endl;

//you can also do that

char AnotherString[256];
strcpy(AnotherString,string);
cout << AnotherString << endl;

delete []string;

This should work fine..

[edited by - Hedos on April 25, 2004 10:46:33 PM]

[edited by - Hedos on April 25, 2004 11:46:12 PM]

Sorry that was inaccurate - I had not initialized the
second string with an amount of data.

char *string1 = "hello!";
char *string2;
strcpy( string2, string1 ); // bugs!!

I did not want to waste extra memory through "char string[64]",
and needed several instances of the same string. The program
is a generic parser and will eventually be used for a
programming language, so I need the strings to be stored in
symbol tables etc.
At the moment my program reads mesh file data from a
text file and uses the string to load bitmaps ...
I suppose the extra NULL entries at the end of the 64 byte
string would not matter when loading the .bmp files! I'll try
that ...

[edited by - ray_intellect on April 26, 2004 4:31:03 AM]

quote:

---

Original post by ray_intellect
>Fixed!!!

>The source below compiles and runs fine. The solution is
>to use a pointer to a pointer when more than one pointer
>to the same data is required!

---

That''s not fixed, it''s basically the same as above, you''re just being lucky. You''re still deleting the same memory twice.

You could try this string class that comes with the c++ standard library.

#include <string>
std::string str1 = "a word";
std::string str2;
str2 = str1;
 

Or read up more about pointers and strings.

---

Why you shouldn''t use iostream.h - ever! | A Good free online C++ book

quote:

---

Original post by Endurion


That''s not fixed, it''s basically the same as above, you''re just being lucky. You''re still deleting the same memory twice.

---

okay, so the soluion must be as below:

	if( str2 ) // is the address of the pointer =to NULL?

	{
		delete [] *str2;
		str2 = NULL;
	}

/// replaced with


	if( *str2 ) // since the value in str1 is NULL 

	{
		delete [] *str2;
		str2 = NULL;
	}

both worked, and debugger did not notice.
btw. The use of a char string[64] does not work and I get an
access violation error from strcat.asm

src_misaligned:                     ; simple byte loop until string is aligned
        mov     dl,byte ptr [ecx]
        inc     ecx
        test    dl,dl
        je      short byte_0
        mov     [edi],dl
        inc     edi
        test    ecx,3
        jne     short src_misaligned
        jmp     short main_loop_entrance

I don''t know what is happening here!

quote:

---

Original post by ray_intellect
okay, so the soluion must be as below:

	if( str2 ) // is the address of the pointer =to NULL?

	{
		delete [] *str2;
		str2 = NULL;
	}

/// replaced with


	if( *str2 ) // since the value in str1 is NULL 

	{
		delete [] *str2;
		str2 = NULL;
	}


[/quote]

No matter how you twist it, you CANNOT call delete[] twice without newing twice. You new once and then call delete[] twice. Just another variant. The first time you call delete[] the memory is freed and MUST NOT be touched anyhow.

>both worked, and debugger did not notice.
That''s a great quote, maybe the debugger didn''t notice but someone using your game/app probably will.

quote:

---

Original post by Endurion
No matter how you twist it, you CANNOT call delete[] twice without newing twice. You new once and then call delete[] twice. Just another variant. The first time you call delete[] the memory is freed and MUST NOT be touched anyhow.

>both worked, and debugger did not notice.
That's a great quote, maybe the debugger didn't notice but someone using your game/app probably will.

---

Well, in this case it will actually work since he's NULLing str1 and thus *str2 = NULL, too, and the second delete[] will never be called. The solution looks a bit odd, though.

quote:

---

char *string1 = "hello!";
char *string2;
strcpy( string2, string1 ); // bugs!!

---

This won't work because string2 points to no valid memory location. This should work instead:

char* string1 = "hello!";
char* string2 = new char[strlen(string1)+1];  // one extra byte for null character

strcpy (string2, string1);
//...

delete[] string2;
string2 = 0;

But since you're using C++ I'll also suggest you learn using std::string. It's just so much less pain.

EDIT: The tags hate me

[edited by - nonpop on April 26, 2004 7:55:09 AM]

[edited by - nonpop on April 26, 2004 7:56:18 AM]

[edited by - nonpop on April 26, 2004 7:57:24 AM]

quote:

---

Original post by nonpop

quote:

---

Original post by Endurion
No matter how you twist it, you CANNOT call delete[] twice without newing twice. You new once and then call delete[] twice. Just another variant. The first time you call delete[] the memory is freed and MUST NOT be touched anyhow.

>both worked, and debugger did not notice.
That's a great quote, maybe the debugger didn't notice but someone using your game/app probably will.

---

Well, in this case it will actually work since he's NULLing str1 and thus *str2 = NULL, too, and the second delete[] will never be called. The solution looks a bit odd, though.

---

It is true that delete should "fall through" on a zero value.
The SDK documentation states:

quote:

---

Using delete on a pointer to an object not allocated with new gives unpredictable results. You can, however, use delete on a pointer with the value 0. This provision means that, because new always returns 0 on failure, deleting the result of a failed new operation is harmless.

---

The DirectX API makes extensive use of a SAFE_DELETE macro
which goes like

#define SAFE_DELETE(p) if(p){delete p; p = NULL;}

and that is the pattern I was trying to recreate with the
strings.
I think that the problem in the original case was I forgot to
treat str2 as an "instance" of a memory address. Checking the
address of a NULLed pointer should not work technically, but
then I believe dereferencing the str2 on the call to delete[]
would be like calling a NULL value.

B4 I say more I will have to try the "new char[strlen(str1)+1]"
call, because I was assuming strlen would include the NULL.




(edit) quote tags



[edited by - ray_intellect on April 26, 2004 8:39:23 AM]

[edited by - ray_intellect on April 26, 2004 8:51:16 AM]

whew - I've got the entire thing fixed. The actual code is
something like

class MeshMaterial
{
public:
    MeshMaterial(){}
    ~MeshMaterial()
    {
    /*  old code
        if( *strBmp )
        {
            delete [] *strBmp;
            strBmp = NULL;
        }
    */
    }
    // ...


    char **strBmp;

};

// ...


void MeshMaterial::SetTextureName( Token &t )
{
    strBmp = &t.word
}

//...


HRESULT Init()
{
    Parser p;
    p.LoadInputFile( "scene1.txt" );
    PostProcessScene( p );
    // p.ParserCleanup(); // old cleanup of tokens (with strcpy)


    // ...

    // load textures using strBmp

    // ...


    p.ParserCleanup(); // now here! free all tokens


    // strBmp is now pointing to NULL and does not need deletion


    // ...

    return S_OK;
}

Now I can begin with the painstaking process of converting to
std::string

(edit) spelling error

[edited by - ray_intellect on April 26, 2004 9:54:03 AM]

[edited by - ray_intellect on April 26, 2004 9:59:53 AM]

[edited by - ray_intellect on April 26, 2004 10:06:35 AM]