problem with delete[]

[B.S]

helo all, I have a problem with delete[]. after debugging of program
it givs mi error whith abort, retry and ignor.

here is code

if(LOWORD(wParam)==IDC_B1)
{
TCHAR *text=new TCHAR[256];

// Set some text

delete[] text;
}

[Todd Aspeotis]

That code will work fine as is. It's the // Set some text part that is
probably killing you, what do you do?

"B.S" <giob...@gmail.com> wrote in message
news:46aa2246-568e-4e97...@a21g2000yqc.googlegroups.com...

[B.S]

if(LOWORD(wParam)==IDC_B1)
{
TCHAR *text=new TCHAR[256];

text=(TCHAR *)FillOpenParams(hwnd,OPEN);
SetDlgItemText(hwnd,IDC_EDIT1,text);

delete[] text;
}

FillOpenParams is returning plaithment of file. i tryed without delete
[] text and it worked fine but with it it crushes always.

[Timo Kunze]

> FillOpenParams is returning plaithment of file. i tryed without delete
> [] text and it worked fine but with it it crushes always.

You allocate some memory and set 'text' to the address of this memory.
Immediately after that, you set 'text' to the result of FillOpenParams.
Now you don't know the address of the allocated memory anymore and
therefore can't free it and so this memory is leaked.
This was problem #1 with your code. Problem #2 is, that you can't use
delete[] to free memory that has not been allocated using new[]. So look
how FillOpenParams allocates the memory of which it returns the address
and use the appropriate method to free this memory - if it must be freed
at all.

Timo
--
www.TimoSoft-Software.de - Unicode controls for VB6
"Those who sacrifice freedom for safety deserve neither."
"Demokratie ist per Definition unsicher. Ihr Schutz entsteht aus der
Überzeugung, dass die demokratischen Kräfte überwiegen und sich – auf
demokratischem Wege – durchsetzen."

[ScottMcP [MVP]]

On Nov 21, 6:22 am, "B.S" <giobs...@gmail.com> wrote:
>  text=(TCHAR *)FillOpenParams(hwnd,OPEN);

You are probably hoping that line copies text into the text array, but
it does not. It copies a pointer into the text pointer. So the
original text pointer is erased, which is why the later delete []
fails.

What is the type that is returned by FillOpenParams and why are you
casting it to TCHAR*? Nobody can show you how to fix it without that
missing information.

[B.S]

This is code of FillOpenParams:

char *FillOpenParams( HWND hwnd, int ind)
{
OPENFILENAME open_params = {0};
char filter[128] = {0};
char file_name[512] = {0};

strcat(filter,"All File");
int index = strlen(filter) + 1;

filter[index++] = '*';
filter[index++] = '.';
filter[index++] = '*';

open_params.lStructSize = sizeof(OPENFILENAME);
open_params.hwndOwner = hwnd;
open_params.lpstrFilter = filter;
open_params.lpstrFile = file_name;
open_params.nMaxFile = 1024;
open_params.lpstrInitialDir = NULL;
open_params.lpstrFileTitle = NULL;
open_params.Flags = OFN_FILEMUSTEXIST | OFN_PATHMUSTEXIST ;

if (ind==OPEN)
{
if(!GetOpenFileName(&open_params)) {return 0;}
return open_params.lpstrFile;
}

if (ind==SAVE)
{
if(!GetSaveFileName(&open_params)) {return 0;}
return open_params.lpstrFile;
}

else {MessageBox(hwnd,"unnown identifier",0,0);}
}

[Timo Kunze]

B.S schrieb:

> char *FillOpenParams( HWND hwnd, int ind)

You should not cast char* to TCHAR*. For Unicode builds, a TCHAR is
something different than a char and a typecast doesn't convert an ANSI
string into a Unicode string. With this typecast, the compiler won't
warn you about this problem.

> char file_name[512] = {0};

> open_params.lpstrFile = file_name;
> return open_params.lpstrFile;
The memory occupied by 'file_name' is freed when the function is left,
so you are returning the address to a memory block that no longer will
be valid when it is accessed. Access violation I here you coming...

[Friedel Jantzen]

Use an extra parameter and copy the string:

bool FillOpenParams(HWND hwnd, int ind, char *text)
{

.....
open_params.nMaxFile = MAX_PATH;

.....

if (ind==OPEN)
{
if(!GetOpenFileName(&open_params)) {return false;}
strcpy(text, open_params.lpstrFile); // copy to the buffer of the caller
return true;
}

// change other code likewise

........

}

// caller
if(LOWORD(wParam)==IDC_B1)
{
char *text=new char[MAX_PATH];

// Set some text
if(FillOpenParams(hwnd, ind, text)) DoSomething();

delete[] text;
}

[Timo Kunze]

Friedel Jantzen schrieb:

> bool FillOpenParams(HWND hwnd, int ind, char *text)

This is prone to buffer overflows. The function doesn't know the size of
'text' and can only guess how many characters this buffer can take. The
buffer size should be passed, too.

> strcpy(text, open_params.lpstrFile); // copy to the buffer of the caller

This also is prone to buffer overflows. Strcpy copies until it finds a
null character. It doesn't take the buffer size into account. It's
better to use lstrcpyn here or even better: StringCchCopy.

[Friedel Jantzen]

Am Sun, 22 Nov 2009 10:55:46 +0100 schrieb Timo Kunze:

> Friedel Jantzen schrieb:
>> bool FillOpenParams(HWND hwnd, int ind, char *text)
> This is prone to buffer overflows. The function doesn't know the size of
> 'text' and can only guess how many characters this buffer can take. The
> buffer size should be passed, too.
>
>> strcpy(text, open_params.lpstrFile); // copy to the buffer of the caller
> This also is prone to buffer overflows. Strcpy copies until it finds a
> null character. It doesn't take the buffer size into account. It's
> better to use lstrcpyn here or even better: StringCchCopy.
>
> Timo

You're right. StringCchCopy is recommended. And will work, if you input the
correct max length ;-)
If I know that the source string will fit into the dest. buffer, is it
necessary to do extra checking?

If the user can select only a single file, as the OP posted,
and
open_params.nMaxFile = MAX_PATH;
can the resulting string be longer than MAX_PATH?

Regards,
Friedel

[Timo Kunze]

Well, such limitations may be lifted as time moves on and then you'll be
glad to have written the code in a defensive way. ;)
You're right in this specific case. But if you don't avoid strcpy in
general, time will come when you make a failure and decide for strcpy
when you really should use lstrcpyn. I prefer to always use at least
lstrcpyn. It may be less performant, but it's also less likely I make a
failure that may result in a buffer overflow.
You really should pass FillOpenParams the buffer size. Or how do you
make sure that you never call this function from another part of your
code, with a buffer smaller than MAX_PATH chars?

[Friedel Jantzen]

Am Sun, 22 Nov 2009 21:57:29 +0100 schrieb Timo Kunze:

> Well, such limitations may be lifted as time moves on and then you'll be
> glad to have written the code in a defensive way. ;)

I agree, that the defensive way is important to write robust software,
and admit using StringCchCopy very often. IMO especially important on user
input or reading from files, when I do not know the length of the strings.

> You're right in this specific case. But if you don't avoid strcpy in
> general, time will come when you make a failure and decide for strcpy
> when you really should use lstrcpyn. I prefer to always use at least
> lstrcpyn. It may be less performant, but it's also less likely I make a
> failure that may result in a buffer overflow.

I used lstrcpyn for a long time, but switched to StringCchCopy some time
ago.

> You really should pass FillOpenParams the buffer size. Or how do you
> make sure that you never call this function from another part of your
> code, with a buffer smaller than MAX_PATH chars?

In this case I am sure; for single selection file open dialogs I always
provide [MAX_PATH] sized buffers. For my purposes I have written some well
tested standard functions and C++ classes for open/save file dialogs.
However in my posting I tried to reuse the code of the OP as much as
possible (it could be improved...).

Regards,
Friedel

[B.S]

Thanks to all for help and cant you tell me where can i find more
about optimising progams