Problems with VC++

[TheCurious08]

I have the following code in VC++:

In TestView.h :

/***********************************/
......
.....
typedef struct t_test
{
int a, b;
struct t_test *next;
} Obj;

class CTestView : public CView
{
public :
Obj *OO;
Obj Make(int aa, int bb);
void Read();
void foo();
int N;
.......
}


In TestView.cpp the functions are as follows :

Obj CTestView::Make(int aa, int bb)
{
Obj *tmp;

tmp = (Obj *)malloc(sizeof(Obj));
tmp->a = aa;
tmp->b = bb;
}

void CTestView::Read()
{
Obj *new_o;

new_o = &Make(1, 2);
new_o->next = OO;
OO = new_o;

new_o = &Make(3, 4);
new_o->next = OO;
OO = new_o;
}

void CTestView::OnLButtonDown(.....) // or any other function
}
Read();
foo();
}

/************************************/

Thats is the essence of the code. My problem is :

In Read() the values for OO are correctly assigned. But after the function Read() is left and enters in foo() (OnLButtonDown) the values of OO are completely change (other values for a and b and next is not accessible).

What's wrong ? How can you correct ?

Thank you. The Curious 08

[r035198x]

Please don't flood the forum with the same question. Read the posting guidelines before posting further.

[Banfa]

CTestView::Make does not return anything.

I assume in your compiling code it returns something like *tmp. This is a mistake the compiler now creates a temporary object to return from the function (and the memory you allocated in malloc is leaked as it is never freed). You store a pointer to the temporary object in CTestView::Read but the object is temporary and is destroyed at the end of the assignment so then the pointer is pointing an invalid memory location.

You access this in foo and get undefined behaviour.

You should return Obj * from CTestView::Make and pass the pointer allocated by malloc back to the calling function.

And finally this is C++, you should prefer using new over malloc unless you have a valid reason that you can write down on paper why you need to use malloc (not being familiar with new is not a valid reason).

[TheCurious08]

CTestView::Make does not return anything.

I assume in your compiling code it returns something like *tmp. This is a mistake the compiler now creates a temporary object to return from the function (and the memory you allocated in malloc is leaked as it is never freed). You store a pointer to the temporary object in CTestView::Read but the object is temporary and is destroyed at the end of the assignment so then the pointer is pointing an invalid memory location.

You access this in foo and get undefined behaviour.

You should return Obj * from CTestView::Make and pass the pointer allocated by malloc back to the calling function.

And finally this is C++, you should prefer using new over malloc unless you have a valid reason that you can write down on paper why you need to use malloc (not being familiar with new is not a valid reason).

Hi

Thank for your reply. I have missed to add "return (*tmp);"in the message but in the program it appears. Bad habit with malloc(). I really don't know how to solve the problem.

The Curious

[JosAH]

Thank for your reply. I have missed to add "return (*tmp);"in the message but in the program it appears. Bad habit with malloc(). I really don't know how to solve the problem.

A bad habit? It is plain wrong. Return the pointer to the malloc'd struct, keep it
somewhere and work with what the pointer points to and free() it afterwards.

kind regards,

Jos