C / C++ / MFC

7-Sep-07 15:06

Hello all,
I'm having a problem when trying to free a string that is pointed from a member of colName array.
That is of course after I'm allocation memory for the colName array, and each of it's members.

the struct i'm working on:

typedef struct
{
	char** colName;
	const struct element_interface** ifc;
	Element* temp_row;
}data;

typedef struct table_rec
{
	char TabInitFlag;
	int iterator;
	data* localData;
	char** last_found_array;
	Matrix tableArray;
	int tableWidth;
	int last_found;
}table_rec;

The command that I'm using to free the string.

int num; //the number of members at colNum
for(i = 0; i < num; i++)
{
	free(colNum[i]);
}

The error I'm receiving in the first iteration (as any):
Unhandled exception at 0x00418780 in project.exe: 0xC0000005: Access violation reading location 0xfdfdfe01.

Thank you,
Gizmokaka

Leslie Sanford7-Sep-07 15:28

Leslie Sanford

7-Sep-07 15:28

gizmokaka wrote:
That is of course after I'm allocation memory for the colName array, and each of it's members.

Just to confirm, you are allocating memory for the pointer that points to an array of pointers, correct? Something like:

char **myArray = malloc(sizeof(char *) * 10);
int i;
 
for(i = 0; i < 10; i++)
{
    myArray[i] = malloc(sizeof(char) * 15);
}

gizmokaka7-Sep-07 21:51

7-Sep-07 21:51

yes that is just about how I allocated the space.

John R. Shaw7-Sep-07 17:30

7-Sep-07 17:30

Wow! I am glad that was not a physical blow. (Just messing with you Wink | ;)

)

First the ‘data’ structure needs values that tells us how many items are in each array contained in the structure, otherwise we will not know how many to free.

The “data” structure contains an array of names (colName), when freeing the names you need to ensure that the pointer to the array and that each pointer to a name is valid or you will get an error (if you are lucky) or unexpected results.

To free the array do the following:
1) Check that ‘colName’ is not NULL; write an initialization function to ensure that it is NULL when it is created, similar to a C++ constructor method. You could just write “data mydata = {0}” when declaring a ‘data’ type and all members will be set to zero (NULL).
2) Before calling ‘free’ for an individual item, make sure that it is not pointing to NULL, before freeing it.
3) After calling ‘free’, set the item (colName[i]) to NULL, so you will not try to free it a second time (by accident).
4) After freeing the individual items, you need to free ‘colName’ and set it to NULL, because it not longer points to any valid data.

Example:

if( data.colName ) /* colName != NULL */
{
    int i;
    for( i=0; i < data.NameCount; ++i )
    {
        if( data.colName[i] ) /* data.colName[i] != NULL */
        {
            free(data.colName[i]);
            data.colName[i] = NULL;
        }
    }
    free(data.colName);
    data.colName = NULL;
}

If you understand all of that, then welcome to the world of real programmers!

P.S. I am ignoring the fact that your example is freeing something called colNum, which does not exist.

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

gizmokaka7-Sep-07 22:01

7-Sep-07 22:01

Hello john and thanks for the attention, never the less, you did not supply me with any new information, for things I didn't do , nor was your answer "problem oriented" because it didn't address my problem.

my problem is specific, I am declaring a char** pointer inside my struct that 's purpose is to hold an array of chars in the near future.
during run time I am initializing it with values(pointers to strings).
now in one of my functions I am supposed to free the memory each of the members take, and then the pointer itself.

//t = a pointer to table_rec
//local_data = pointer to data 
for(i = 0 ; i < width ; i++)
{
    free(t->local_data->colName[i]);
}
free(t->local_data_colName);

still for some odd reason I get the error already posted.

John R. Shaw7-Sep-07 22:34

7-Sep-07 22:34

Actually it did address your problem for the information you gave. If it was new information, I have no idea.

You were trying to free an invalid pointer (probably NULL), so how was it not “problem oriented”?

The new code you posted suffers from some of the problems I told you about, but if you do not call your personal free function twice it will be no problem. Otherwise you will get the same errors you originally posted about.

I know C inside and out – if you do not understand the answer, then I can not help you.

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

gizmokaka7-Sep-07 23:05

7-Sep-07 23:05

Look I am watching my code, and I do not try to free NULL pointers.

then again if you see something in my code that is not O.K please post a code snippet that shows the problem so I can understand exectly where i am falling.
I posted a simple example in a main func.
may threw looking at that you can explain exactly what's wrong.

I appreciate what ever help you can give me.

John R. Shaw7-Sep-07 23:42

7-Sep-07 23:42

The original code snippet I posted shows everything you need to know about freeing that type of array. What the rest of your code is doing, I have no clue.

The only thing wrong with your second post of code was that it did not check if any of the pointers where NULL before freeing them, nor did you set them to NULL after freeing them. Of course that was basically the problem with you first post of code.

I have not seen, nor want to, the rest of your code, but you should also make sure that any slot (colName[0] is a slot) is empty (points to NULL) before assigning a new pointer to it, or you will again have memory leaks.

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

Leslie Sanford7-Sep-07 22:34

Leslie Sanford

7-Sep-07 22:34

Ok, you've allocated the necessary memory for the struct and its members. But when you try to access the pointers to free the memory they point to, you get an access violation. That tells me that maybe you're losing track of the pointers somewhere along the way.

I'd suggest putting in asserts in your code to make sure the pointers aren't null. Something like:

assert(t != NULL);
assert(t->local_data != NULL);
assert(t->locat_data->colName != NULL);
 
for(i = 0 ; i < width ; i++)
{
    assert(t->local_data->colName[i] != NULL);
     
    free(t->local_data->colName[i]);
}
 
free(t->local_data_colName);

Backtrack from this location to the one in which you originally allocated memory. Liberally place asserts along the way to make sure that the pointers don't get lost. Hopefully this till catch where the problem is originating.

John R. Shaw7-Sep-07 22:54

7-Sep-07 22:54

That is actually a good idea, but look at your assert. It is a good idea but if it hits, then it just shows that an ‘if’ statement should be there. So if the assert complains, then something is wrong because the same function was called twice without t->local_data_colName being deleted (or set to an invalid value – most likely).

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

gizmokaka7-Sep-07 22:34

7-Sep-07 22:34

#include <stdio.h>
#include <string.h>
#include <malloc.h>


void main ()
{
	int num = 2;
	char** str_array;
	char str1[] = "first string";
	char str2[] = "second string";

	str_array  = (char**)malloc(sizeof(char*)*num);
	str_array[0] = (char*)malloc(sizeof(char)*strlen(str1));
	str_array[1] = (char*)malloc(sizeof(char)*strlen(str2));
	strcpy (str_array[0],str1);
	strcpy (str_array[1],str2);
	
	printf("%s \n",str_array[0]);
	printf("%s \n",str_array[1]);

	free(str_array[0]);//error ocurres
	free(str_array[1]);
	free(str_array);


}

Please help me spot the mistake.

John R. Shaw7-Sep-07 22:42

7-Sep-07 22:42

LOL – That would actually work. It makes no since, but it would work.

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

markkuk7-Sep-07 23:12

markkuk

7-Sep-07 23:12

gizmokaka wrote:

str_array[0] = (char*)malloc(sizeof(char)*strlen(str1));
str_array[1] = (char*)malloc(sizeof(char)*strlen(str2));
strcpy (str_array[0],str1);
strcpy (str_array[1],str2);

You aren't allocating memory for the terminating /0 in the strings, which causes strcpy() to overwrite memory outside the allocated block. Depending on the implementation, that may trash the metadata used by malloc()/free() and cause an error.

gizmokaka7-Sep-07 23:21

7-Sep-07 23:21

Markkuk

Thanks man it's just what I was looking for.

I probably will address you with further questions later on,

good day.

John R. Shaw7-Sep-07 23:24

7-Sep-07 23:24

lol - I missed that

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

Re: Another qestion on the same subject.

Leslie Sanford7-Sep-07 23:42

Leslie Sanford

7-Sep-07 23:42

Good catch.

gizmokaka8-Sep-07 0:20

8-Sep-07 0:20

Element = void*
I'm experiencing the same problem in a somewhat different situation
Here is the relevant code:

Element string_to_element(const char* val)
{
	Element localElem = malloc(sizeof(val));
	strcpy((char*)localElem , val);
	return localElem;
}

const char* element_to_string(Element e)
{
	int i;
	const char* localStr = (char*)malloc(sizeof(char)*(strlen((char*)e)+1));
	
	for (i = strlen(localStr) ; i > 0 ;i--)
		((char*)e)[i] = NULL;

	strcpy(localStr , (char*)e);
	return localStr;
}
void string_free_func(Element strElem)
{
	char* string;
	if(strElem == NULL)
        return;
	string = (char*)strElem;
	free(string); //error occurs 

}


void main()
{
	char* str2;
	char* str1 = "simple string";
	Element e1 , e2;
	
	e1 = string_to_element(str1);
	str2 = element_to_string(e1);
	
	string_free_func(e1);
}

This code is a bit modified but should illustrate the problem I'm facing.
any help would be most appreciate

Russell^'7-Sep-07 23:33

Russell^'

7-Sep-07 23:33

Be care with pointers! Big Grin | :-D

void main ()
{
	int num = 2<code>, i</code>;
	char** str_array;
	char str1[] = "first string";
	char str2[] = "second string";

	<code>str_array=NULL;</code>
	str_array  = (char**)malloc(sizeof(char*)*num);
	<code>for(i=0;i<num,i++) str_array[i]=NULL;</code>
	str_array[0] = (char*)malloc(sizeof(char)*strlen(str1));
	<code>for(i=0;i<strlen(str1),i++) str_array[0][i]=NULL;</code>
	str_array[1] = (char*)malloc(sizeof(char)*strlen(str2));
	<code>for(i=0;i<strlen(str2),i++) str_array[1][i]=NULL;</code>
	strcpy (str_array[0],str1);
	strcpy (str_array[1],str2);
	
	printf("%s \n",str_array[0]);
	printf("%s \n",str_array[1]);

	<code>if(str_array[0]!=NULL)</code>
		free(str_array[0]);
	<code>if(str_array[1]!=NULL)</code>
		free(str_array[1]);
	<code>if(str_array!=NULL)</code>
		free(str_array);
}

Russell

Closing a view

bob169727-Sep-07 11:08

bob16972

7-Sep-07 11:08

What is the best way in MFC to programmatically close a CView derived class instance?

I'm currently posting a WM_CLOSE to the views parent frame window. Is this the common way to do this?

Russell^'7-Sep-07 23:02

Russell^'

7-Sep-07 23:02

Yes.
then the frame calls DestroyWindow to destroy itself. The message will be sended to all the childs, so also to your CView derived class.
Then you have only to write the right code to destroy correctly the view.

Russell

bob169728-Sep-07 4:44

bob16972

8-Sep-07 4:44

Thanks.

I was having a problem elsewhere in the code and I was trying to detemine if this method was sound or if it might mess up the MFC internal houskeeping somehow.

Most of the books I have don't cover this except one which suggested this technique.

Again, thanks for your help.

Mark Salsbery8-Sep-07 7:04

Mark Salsbery

8-Sep-07 7:04

In addition to Russell's reply -

Typically you'd use DestroyWindow() to destroy a window but with MFC doc/view, that
would bypass the prompt to save an unsaved document and delete the document if its
autodelete flag is set.

I disagree with Russell's statement that the WM_CLOSE will be sent to the children, but
it's generally not an issue anyway since only frame windows do something useful with the
message.

I always figured the reason it's not documented is MFC just assumes only the user will
close a window. Just one of the many irritating things about the doc/view architecture.

Cheers,
Mark

Mark Salsbery
Microsoft MVP - Visual C++
Java | [Coffee]