Sign in to follow this  
CoreMeltdown

Multiple serial-dependent allocations

Recommended Posts

Hi everyone, please consider this (maybe frequent) C/C++ code:

/* GLOBALS (or class members, or whatsoever...) */

SomeType1 * pT1 = 0;
SomeType2 * pT2 = 0;
/* ... */
SomeTypeN * pTN = 0;

bool MyInitFuncion(void) {
	pT1 = new SomeType1[/* ..size1.. */];
	if (!Check_Or_Use_T1(pT1)) {
		return (false);
	}

	pT2 = new SomeType2[/* ..size2.. */];
	if (!Check_Or_Use_T2(pT2)) {
		delete pT1;
		return (false);
	}

	/* ... */
	/* ... */
	/* ... */

	pTN = new SomeTypeN[/* ..sizeN.. */];
	if (!Check_Or_Use_TN(pTN)) {
		delete pT1;
		delete pT2;
		/* ... */
		/* ... */
		/* ... */
		return (false);
	}

	return (true);
}



As you can see, this code, thus it seems robust (well, at least to me), could be error-prone and quite unpratical. Here is the most-seen solution i've found:

bool MyInitFuncion(void) {
	pT1 = new SomeType1[/* ..size1.. */];
	if (!Check_Or_Use_T1(pT1)) {
		goto my_error;
	}

	pT2 = new SomeType2[/* ..size2.. */];
	if (!Check_Or_Use_T2(pT2)) {
		goto my_error;
	}

	/* ... */
	/* ... */
	/* ... */

	pTN = new SomeTypeN[/* ..sizeN.. */];
	if (!Check_Or_Use_TN(pTN)) {
		goto my_error;
	}

	return (true);

my_error:
	delete pT1;
	delete pT2;
	/* ... */
	/* ... */
	/* ... */
	delete pTN;

	return (false);
}



But "goto"... quite ugly although functional :P Another solution could be to group each runtime allocated pointer (object or array...) into a class/struct and then call a specific free funcion:

void Clean(MyGlobalStruct * pGlobal) {
	delete pGlobal->pT1;
	delete pGlobal->pT2;
	/* ... */
	/* ... */
	/* ... */
	delete pGlobal->pTN;
}

bool MyInitFuncion(void) {
	my_global_struct->pT1 = new SomeType1[/* ..size1.. */];
	if (!Check_Or_Use_T1(my_global_struct->pT1)) {
		Clean(my_global_struct);
		return (false);
	}

	my_global_struct->pT2 = new SomeType2[/* ..size2.. */];
	if (!Check_Or_Use_T2(pT2)) {
		Clean(my_global_struct);
		return (false);
	}

	/* ... */
	/* ... */
	/* ... */

	my_global_struct->pTN = new SomeTypeN[/* ..sizeN.. */];
	if (!Check_Or_Use_TN(my_global_struct->pTN)) {
		Clean(my_global_struct);
		return (false);
	}

	return (true);
}



Consider these examples with explanatory purpose only (substitute operator new with malloc() and delete with free() or every language-specific memory allocation, though the alloc/free code can be seen as generic initialize/finalize code). Some opinions? Thanks :) [Edited by - CoreMeltdown on July 17, 2005 5:51:54 AM]

Share this post


Link to post
Share on other sites
hi desertcube.
thanks for your reply, but IMO it's not focused on my problem, maybe I wasn't clear enough.
The problem is not in pointers' scope.
Say, you have some piece of data you can initialize/finalize.
You initialize a certain datum only if all previous data have been well initialized. If something goes wrong with your last initialization you have to "finalize" all previously-initialized data.

Share this post


Link to post
Share on other sites
No, my post was related. Basically you let a class's constructor/destructor take care of it for you.

For example, say you were dealing with the Win32 API GlobalAlloc, you may want to encapsulate this into a class as follows:

template <class T>
struct GlobalMemory
{
GlobalMemory() {_storage = (T *)GlobalAlloc(GMEM_FIXED, sizeof(T));}
~GlobalMemory() {GlobalFree(_storage);}

T * operator->() {return _storage;}
private:
T * _storage;
};



HTH

Share this post


Link to post
Share on other sites
Probably the nicest version of your code would use smart pointers and look something like this:
bool MyFunction()
{
boost::shared_array<MyType> p1(new MyType[123]);
if(!CheckAndUse(p1)) return false;
boost::shared_array<MyType> p2(new MyType[123]);
if(!CheckAndUse(p2)) return false;
boost::shared_array<MyType> p3(new MyType[123]);
if(!CheckAndUse(p3)) return false;
return true;
}


See: boost::shared_array


Of course, the bunch of if statements looks like it could be worthy of exceptions too, but that will depend on what you're actually doing.

Share this post


Link to post
Share on other sites
Quote:
Original post by CoreMeltdown
thanks for your reply, but IMO it's not focused on my problem, maybe I wasn't clear enough.
The problem is not in pointers' scope.


Actually, if you use std::auto_ptr, it will be.
std::auto_ptr automatically deletes whatever object it points to, when it goes out of scope:

std::auto_ptr<SomeType> ptr1 = 0, ptr2 = 0, ptrn = 0;

bool MyInitFunc()
{
if (!(ptr1 = new SomeType()))
return false;
if (!(ptr2 = new SomeType()))
return false;
if (!(ptr3 = new SomeType()))
return false;
return true;
}




so when you call 'return', anywhere in your function, all of the auto_ptrs will go out of scope, and delete the object they point to.

edit: beaten to it [wink]

Share this post


Link to post
Share on other sites
Quote:
Original post by swiftcoder
Quote:
Original post by CoreMeltdown
thanks for your reply, but IMO it's not focused on my problem, maybe I wasn't clear enough.
The problem is not in pointers' scope.


Actually, if you use std::auto_ptr, it will be.
std::auto_ptr automatically deletes whatever object it points to, when it goes out of scope:
*** Source Snippet Removed ***
so when you call 'return', anywhere in your function, all of the auto_ptrs will go out of scope, and delete the object they point to.

edit: beaten to it [wink]


yes you're right... but the auto_ptr you used are declared globals..
Maybe I don't catch you well.... but as far as i know your code snippet cause all allocated pointers to remain allocated once the funcion returns..

Share this post


Link to post
Share on other sites
Quote:
Original post by CoreMeltdown
I am sorry...
consider a C version of my code...


So you want to do it in C, then? No classes, no deconstructors, no RAII, no exception safety, no operator new (then why did you use it)?

In that case, one of your methods in your first post will probably do. I'm sure someone with more plain-C experience can suggest the most elegant method.

Share this post


Link to post
Share on other sites
Quote:
Original post by Andrew Russell
Quote:
Original post by CoreMeltdown
I am sorry...
consider a C version of my code...


So you want to do it in C, then? No classes, no deconstructors, no RAII, no exception safety, no operator new (then why did you use it)?

In that case, one of your methods in your first post will probably do. I'm sure someone with more plain-C experience can suggest the most elegant method.


Well, however, even if i claimed a plain C anwer, the auto_ptr doesn't solve the problem.

Your solution

std::auto_ptr<SomeType> ptr1 = 0, ptr2 = 0, ptrn = 0;

bool MyInitFunc()
{
if (!(ptr1 = new SomeType()))
return false;
if (!(ptr2 = new SomeType()))
return false;
...
[/code]

if, for example, the second "if" condition is not satisfied, does not deallocate ptr1

Share this post


Link to post
Share on other sites
Quote:
consider a C version of my code...



bool foo()
{
bool error = false;
int* one = 0;
int* two = 0;
int* three = 0;
int stage = 0;

while (true)
{
one = new int[10];
if (!use_one(one)) {
error = true;
break;
}
stage++;
two = new int[10];
if (!use_two(two)) {
error = true;
break;
}
stage++;
three = new int[10];
is (!use_three(three)) {
error = true;
break;
}
stage++;
break;
}
switch (stage)
{
case 3: delete [] three;
case 2: delete [] two;
case 1: delete [] one;
default:
}

return error;
}




Quote:
if, for example, the second "if" condition is not satisfied, does not deallocate ptr1


Of course not; your auto-pointers are globals! Move them into function scope.

Share this post


Link to post
Share on other sites
Quote:
Original post by CoreMeltdown
Your solution
*** Source Snippet Removed ***

That wasn't my solution, that was swiftcoder's solution and it wouldn't work for arrays anyway. Look at my solution for the correct (C++) answer (as Deyja says, with the smart pointers in function scope). Globals are evil anyway.

Share this post


Link to post
Share on other sites
Quote:
Original post by swiftcoder
*** Source Snippet Removed ***
so when you call 'return', anywhere in your function, all of the auto_ptrs will go out of scope, and delete the object they point to.

Uhmn... No. They're globals (thus they don't go out of scope until the program ends).

Share this post


Link to post
Share on other sites
Quote:
Original post by Andrew Russell
Quote:
Original post by CoreMeltdown
Your solution
*** Source Snippet Removed ***

That wasn't my solution, that was swiftcoder's solution and it wouldn't work for arrays anyway. Look at my solution for the correct (C++) answer (as Deyja says, with the smart pointers in function scope). Globals are evil anyway.


I'm really sorry... it wasn't your solution.

But anyway... your solution has local pointers...
Maybe I wasn't clear, I'll try to explain better, writing a dummy example in C:



typedef struct tagMyStruct {
SomeType1 * pT1;
/* ... */
SomeTypeN * pTN;
} MyStruct;

MyStruct * InitMyStruct(int dummySize) {
MyStruct * ms;

ms = (MyStruct *)malloc(sizeof(MyStruct));
if (ms == 0) {
return (0);
}

ms->pT1 = (SomeType1 *)malloc(dummySize * sizeof(SomeType1));
if (ms->pT1 == 0) {
free(ms);
return (0);
}

/* ... */;
/* ... */;
/* ... */;

ms->pTN = (SomeTypeN *)malloc(dummySize * sizeof(SomeTypeN));
if (ms->pTN == 0) {
/* ... */;
/* ... */;
/* ... */;
free(ms->pT1);
free(ms);
return (0);
}

return (ms);
}



Share this post


Link to post
Share on other sites
Code like this is one strong reason to switch to C++, but there are idioms to deal with it in C.

MyStruct * InitMyStruct(int dummySize) {
MyStruct * ms;
jmp_buf done;
int status;

status = setjmp(done);
if (status != 0) {
/* long jmp here to clear ms */
if (ms->pTN) free(ms->pTN);
/* ... */
/* ... */
/* ... */
if (ms->pT1) free(ms->pT1);
free(ms);
return NULL;
}

ms = (MyStruct *)malloc(sizeof(MyStruct));
if (ms == 0) return NULL;
memset(ms, 0, sizeof(ms));

ms->pT1 = (SomeType1 *)malloc(dummySize * sizeof(SomeType1));
if (ms->pT1 == 0) longjmp(done, -1);

/* ... */;
/* ... */;
/* ... */;

ms->pTN = (SomeTypeN *)malloc(dummySize * sizeof(SomeTypeN));
if (ms->pTN == 0) longjmp(done, -1);

return ms;
}


If you're a hardcore SESE evangelist, you can fold the other return values into the longjmp block.

Share this post


Link to post
Share on other sites
Quote:
Original post by CoreMeltdown
I'm really sorry... it wasn't your solution.

But anyway... your solution has local pointers...
Maybe I wasn't clear, I'll try to explain better, writing a dummy example in C:

*** Source Snippet Removed ***


You still havn't been clear if you want C or C++. Here is the same thing in C++, not using local pointers this time:

struct MyStruct
{
boost::shared_array<MyType> p1;
boost::shared_array<MyType> p2;
boost::shared_array<MyType> p3;
};

std::auto_ptr<MyStruct> MyFunction(size_t dummySize)
{
std::auto_ptr<MyStruct> ms(new MyStruct);

ms->p1(new MyType[dummySize]);
ms->p2(new MyType[dummySize]);
ms->p3(new MyType[dummySize]);

// if(!CheckOrUse(p3)) return std::auto_ptr<MyStruct>();

return ms;
}

You may notice I've not checked the pointers for NULL (as you do in the code I'm working off). If they somehow fail to allocate (or a constructor fails), then it will throw an error (unless you are specificly writing with a non-throwing new).

If you're still doing the CheckOrUse function, I've left in an example use. Alternitivly you may want to catch the std::bad_alloc error and return a NULL auto pointer to match the behaviour of your old system if new fails. Alternitivly, you could have CheckOrUse throw an error if it fails (which matches the behaviour if new failed and could be nicer than returning NULL pointers).



Of course, you could go even more C++'ish, you could do it like the following. I've also left a sample CheckOrUse call in there. In this case, it must throw (being in a constructor). IMO this is the nicest version of the code.

class MyClass
{
boost::shared_array<MyType> p1;
boost::shared_array<MyType> p2;
boost::shared_array<MyType> p3;

public:
MyClass(size_t dummySize)
{
ms->p1(new MyType[dummySize]);
ms->p2(new MyType[dummySize]);
ms->p3(new MyType[dummySize]);

// if(!CheckOrUse(p3)) throw SomeErrorClass();
}
};


Share this post


Link to post
Share on other sites
Quote:
Original post by Andrew Russell
... Chunk of code that returns an auto_ptr ...

Eh, don't do that.

Share this post


Link to post
Share on other sites
Quote:
Original post by Washu
Quote:
Original post by Andrew Russell
... Chunk of code that returns an auto_ptr ...

Eh, don't do that.

Eh, why not? (see)

(note to CoreMeltdown: you could replace std::auto_ptr with boost::shared_ptr if you have problems)

Share this post


Link to post
Share on other sites
Quote:
Original post by Andrew Russell
Quote:
Original post by Washu
Quote:
Original post by Andrew Russell
... Chunk of code that returns an auto_ptr ...

Eh, don't do that.

Eh, why not? (see)

Yes, I know... I also know that not all compilers deal with that properly, hence why I typically avoid it (even though I haven't written anything for such a compiler in a while... but you never know when it will crop up in a production project).
Quote:

(note to CoreMeltdown: you could replace std::auto_ptr with boost::shared_ptr if you have problems)

Or if you want to store it in a container, or use it in any other way not mentioned in the article linked above.

Share this post


Link to post
Share on other sites
What good are these "move semantics" of auto_ptr anyway? Why didn't anyone think to include the equivalent but with "copy semantics" in the standard library as well (and perhaps an array version as well)? Perhaps policy-based design (like std::auto_ptr<typename T, typename AssignmentPolicy>) would be useful here?

Share this post


Link to post
Share on other sites
I am terribly, terribly sorry, just didn't quite think it through.
Given the semantics of std::auto_ptr, though, you could probably declare local auto_ptrs in the function, check for errors at the end of the function, and if there are no errors, pass ownership of the objects to permanent storage, something like this:

MyObject *ob1, *ob2, *obn;

bool MyLoadFunc()
{
std::auto_ptr<MyObject> ptr1, ptr2, ptr3;
if (!(ptr1 = new MyObject()))
return false;
if (!(ptr2 = new MyObject()))
return false;
if (!(ptrn = new MyObject()))
return false;

ob1 = ptr1.release();
ob2 = ptr2.release();
obn = ptrn.release();

return true;
}


i.e. if everything loads successfully, we remove control from the auto_ptrs, and assign them to the global (or whatever) pointers, and then they don't delete the resources at the end of the scope.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
What good are these "move semantics" of auto_ptr anyway? Why didn't anyone think to include the equivalent but with "copy semantics" in the standard library as well (and perhaps an array version as well)? Perhaps policy-based design (like std::auto_ptr<typename T, typename AssignmentPolicy>) would be useful here?

They are good for doing the things such as those that are listed in the link I posted above.

Considering why auto-pointer has move-sematics: When you copy an auto-pointer to another auto-pointer, the contained pointer is moved and the source becomes NULL. Now consider if you were to copy the pointer. The old auto_ptr no longer has ownership (it has moved), so basically it acts like a regular pointer.

And this is the reason that auto_ptr moves, rather than copies. If auto_ptr were to act like a regular pointer, it would not only require an extra flag (and check) in auto_ptr to implement, it is rediculiously more confusing than the copy-is-move sematics of the auto_ptr because now we can have variables of type auto_ptr that may or may not own the object in question. Leaving the programmer to puzzle which one must not be let out of scope.

Of course, if auto_ptr copied both pointer and ownership, then it would be a shared pointer, not an auto pointer.

Share this post


Link to post
Share on other sites
CoreMeltdown,

I think the best solution is your third one. The allocated objects cannot exist independently, so their allocation and deallocation should be done in the same place at the same time.

One other thing -- since the pointers are accessible externally, you should set them to 0 after deleting.

Share this post


Link to post
Share on other sites
blah * a = null;
blah * b = null;
blah * c = null;

bool init()
{
if (a = new blah && a->init())
{
if (b = new blah && b->init())
{
if (c = new blah && c->init())
return true;
delete c;
c = null;
}
delete b;
b = null;
}
delete a;
a = null;
return false;
}


This is how I'd handle it. I'm not 100% sure I understood the question, though, so forgive me if I'm completely OT [grin]

Share this post


Link to post
Share on other sites
I would hesitate to call setjmp a safe idiom to use because it magically jumps across functions/stack frames, messes up signals and in general suffers from the same problems as exceptions.

The standard C way to do it is with a bailout ladder:
err = do1()
if(err < 0)
goto exit
err = do2()
if(err < 0)
goto exit_undo1

return 0;// success

exit_undo1:
undo1();
exit:
return err;


Deyja's code basically does the same thing but goes through gyrations to avoid goto (apparently only for the sake of doing so).

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this