clean exit - suggestions?

[lallous]

Hello,

I have a function like:

void fnc() {
char *mem1, *mem2, *mem3, *mem4;
// lots of code...
// mem1, 2, 3, 4 got allocated
// lots of code and condition checks
if (condition_failed)
{
// blah blah
// free mem1, mem2, mem3, mem4
return;
}

if (condition2_failed)
{
// blah blah
// free mem1, mem2, ...
return;
}

// here the end of routine (clean exit code):
// free mem1, mem2, mem3, mem4
}

Usually, I would use compiler specific solution by putting most of the code
in __try() and the clean exit code in _finally() block then to reach the
clean exit code I would invoke __leave.

Or use lables and then goto clean_exit

Any better way, other than goto or compiler specific solution?

 

[lallous]

use a std::string instead of dynamically allocated character arrays.

#include <string>

void fnc()
{
std::string mem1;
std::string mem2;

mem1 = "Hallo";
mem2 = "World";

// look Ma! I don't have to do anything. mem1 and mem2
// will free the allocated memory all by themselfs
}

Hello,

Your solution is a hint...but I am using such buffers to do file i/o and
cannot really use std::string....

And sometimes I have code like:
fclose(fp1); fclose(fp2); free(mem1); myObj->DeInit(); blah blah...
So you see how the cleanup code is not always solved by using objects...so
my question was how to avoid repeating the cleanup code over and over in
case I want to return in the middle of the function.

 

[Karl Heinz Buchegger]

Hello,

I have a function like:

void fnc() {
char *mem1, *mem2, *mem3, *mem4;
// lots of code...
// mem1, 2, 3, 4 got allocated
// lots of code and condition checks
if (condition_failed)
{
// blah blah
// free mem1, mem2, mem3, mem4
return;
}

if (condition2_failed)
{
// blah blah
// free mem1, mem2, ...
return;
}

// here the end of routine (clean exit code):
// free mem1, mem2, mem3, mem4
}

Usually, I would use compiler specific solution by putting most of the code
in __try() and the clean exit code in _finally() block then to reach the
clean exit code I would invoke __leave.

Or use lables and then goto clean_exit

Any better way, other than goto or compiler specific solution?

use a std::string instead of dynamically allocated character arrays.

#include <string>

void fnc()
{
std::string mem1;
std::string mem2;

mem1 = "Hallo";
mem2 = "World";

// look Ma! I don't have to do anything. mem1 and mem2
// will free the allocated memory all by themselfs
}

 

[Hendrik Belitz]

Hello,

I have a function like:
...

Usually, I would use compiler specific solution by putting most of the
code in __try() and the clean exit code in _finally() block then to reach
the clean exit code I would invoke __leave.

Or use lables and then goto clean_exit

Any better way, other than goto or compiler specific solution?

Putting your function into a class and writing an additional member function
cleanup() to close files and delete the buffers would help. Addionally you
could write a class (called eg. Buffer) to encapsule your char buffers and
use auto_ptr<Buffer> in your code then.

 

[Peter van Merkerk]

I have a function like:

void fnc() {
char *mem1, *mem2, *mem3, *mem4;
// lots of code...
// mem1, 2, 3, 4 got allocated
// lots of code and condition checks
if (condition_failed)
{
// blah blah
// free mem1, mem2, mem3, mem4
return;
}

if (condition2_failed)
{
// blah blah
// free mem1, mem2, ...
return;
}

// here the end of routine (clean exit code):
// free mem1, mem2, mem3, mem4
}

Usually, I would use compiler specific solution by putting most of the code
in __try() and the clean exit code in _finally() block then to reach the
clean exit code I would invoke __leave.
Or use lables and then goto clean_exit

Any better way, other than goto or compiler specific solution?

The problem with code you posted is code duplication (= maintenance
headache, which will no doubt lead to bugs later) and that it is not
exception safe.

If it is just freeing char* that is bothering you, why not use the
std::string class?

#include <string>
void fnc()
{
std::string mem1, mem2, mem3, mem4;

// lots of code...
// lots of code and condition checks
if (condition_failed)
{
// blah blah
// No need to explicitly free mem1, mem2, mem3, mem4!
return;
}

if (condition2_failed)
{
// blah blah
// No need to explicitly free mem1, mem2, mem3, mem4!
return;
}

// No need to explicitly free mem1, mem2, mem3, mem4!
}

The advantage of this is not only that all memory used by the string is
automatically released, no matter how the function exits (return or
exception), but also that string manipulation with the std::string is much
easier to code and understand. Similary the std::vector class is an
excellent replacement for dynamically created arrays; also with this class
there is no need to worry about explicit memory management.

For other tasks that need to be done when a functions exits, consider using
the RAII idiom
(http://c2.com/cgi/wiki?ResourceAcquisitionIsInitialization). An example of
RAII is the std::auto_ptr class. The basic idea of the RAII idiom is to
create a local object which has a destructor that perfoms the operations
necessary when the function is exited (the destructor essentially replaces
the non-standard _finally clause). Since the destructor of local objects is
always called when the function exists, the destructor can perfom the
things you now do in the non-standard _finally() clause.

#include <iostream>
class AutoCleanup
{
public:
AutoCleanup();
~AutoCleanup()
{
std::cout << "AutoCleanup dtor" << std::endl
}
}

void fnc()
{
AutoCleanup tmp;
// lots of code and condition checks
if (condition_failed)
{
// The return statement causes
// AutoCleanup::~AutoCleanup to be called
return;
}

if (condition2_failed)
{
// The return statement causes
// AutoCleanup::~AutoCleanup to be called
return;
}

// When the function finishes
// AutoCleanup::~AutoCleanup will be called
}

Also when a exception is thrown the AutoCleanup::~AutoCleanup will be
called. The RAII idiom is a very handy technique which usefullness is not
restricted to releasing memory or other resources. For example for UI code
you could make a class the changes the mouse cursor to a hourglass on
construction and restores the previous mouse cursor on destruction:

class HourGlassCursor
{
public:
HourGlassCursor() : prev_(getCursor())
{
setCursor(crHourGlass);
}

~HourGlassCursor()
{
setCursor(prev_);
};

private:
Cursor prev_;
};

void calculate()
{
HourGlassCursor hgc;
// do some lengthy processing...
}

The cursor will always be restored, even if an exception is thrown.

 

[Jeff Schwab]

Hello,

I have a function like:

void fnc() {
char *mem1, *mem2, *mem3, *mem4;
// lots of code...
// mem1, 2, 3, 4 got allocated
// lots of code and condition checks
if (condition_failed)
{
// blah blah
// free mem1, mem2, mem3, mem4
return;
}

if (condition2_failed)
{
// blah blah
// free mem1, mem2, ...
return;
}

// here the end of routine (clean exit code):
// free mem1, mem2, mem3, mem4
}

Usually, I would use compiler specific solution by putting most of the code
in __try() and the clean exit code in _finally() block then to reach the
clean exit code I would invoke __leave.

Or use lables and then goto clean_exit

Any better way, other than goto or compiler specific solution?

Put the cleanup code in the destructor of an automatic variable. The
resource acquisition probably should be done in a member initialization
list, too. Google. for RAII.

#include <cstddef>

template< typename T >
struct Mem
{
Mem( size_t z ): p( new T [ z ] ) { }

~Mem( ) { /* "blah blah," as you say. */ delete [ ] p; }

T* p;
private:
Mem( Mem const& ) { /* Don't copy the Mem objects. */ }
};

void fnc( )
{
// lots of code...

Mem< char > mem1, mem2, mem3, mem4;

// lots of code and condition checks

if (condition_failed)
{
return;
}

if (condition2_failed)
{
return;
}

// here the end of routine
}

 

[Donovan Rebbechi]

news:[email protected]...

Your solution is a hint...but I am using such buffers to do file i/o and
cannot really use std::string....

And sometimes I have code like:
fclose(fp1); fclose(fp2); free(mem1); myObj->DeInit(); blah blah...

So use your own buffer class that wraps a pointer to the dynamic string:

class buf {
char* mem;
public:
buf():mem(NULL) {}
buf (const char* x, int len) : mem (new char[len]) {
copy(x,x+len,mem);
}
void clear() { delete[] mem; mem = NULL; }

~buf() { delete [] mem; }
};


class Foo
{
FILE * fp1, *fp2;
auto_ptr<myObj> myObj;
Foo& operator=(const Foo&); // disable copy construct and assign
Foo(const Foo&);
public:
~Foo() {
fclose(fp1); fclose(fp2); buf.clear(); myObj->DeInit();
...
...
};

If you want your code to be exception safe, it would probably be a good idea
to wrap your file pointer in a class that handles closing the files too.

So you see how the cleanup code is not always solved by using objects...so

Yes it is. You just need the right object.

Cheers,

 

[Karl Heinz Buchegger]

Hello,

Your solution is a hint...but I am using such buffers to do file i/o and
cannot really use std::string....

then use something else which has the same semantics (destroy dynamically
allocated memory when the object goes out of scope)

#include <vector>

void fnc()
{
std::vector<unsigned char> mem1;
std::vector<unsigned char> mem2;

mem1.resize( 200 ); // tell mem1 that I need 2 unsigned chars
mem2.resize( 500 ); // and mem2 will need 500 unsigned chars

mem1[20] = 5;
mem2[80] = 8;

// look Ma! I don't have to do anything. mem1 and mem2
// will free the allocated memory all by themselfs
}

And sometimes I have code like:
fclose(fp1); fclose(fp2); free(mem1); myObj->DeInit(); blah blah...
So you see how the cleanup code is not always solved by using objects...

It can *always* be solved by putting the thing you need to manage into
a class of its own. In fact that is the whole point of the RAII
idiom. The destructor gets called when the object goes out of scope
no matter how this is achieved. And the destructor is responsible for
deleting the dynamically managed resource (be it some memory, or a file
handle, or a modem or whatever).

so
my question was how to avoid repeating the cleanup code over and over in
case I want to return in the middle of the function.

Encapsulate the resource in a class and put the cleanup code into the destructor.
For some common types of resources this has already be done in your library,
eg std::string, std::vector, std::map, io streams, etc..

 

[tom_usenet]

Your solution is a hint...but I am using such buffers to do file i/o and
cannot really use std::string....

And sometimes I have code like:
fclose(fp1); fclose(fp2); free(mem1); myObj->DeInit(); blah blah...

shared_ptr<FILE> fp1(fopen("Filename", "r+"), fclose);

So you see how the cleanup code is not always solved by using objects...so
my question was how to avoid repeating the cleanup code over and over in
case I want to return in the middle of the function.

Use some kind of scope guard class. boost::shared_ptr is good for most
uses (particularly when combined with boost::bind and boost::mem_fn).
You might prefer an alternative such as:

http://www.cuj.com/documents/s=8000/cujcexp1812alexandr/

Note that there should soon be std::shared_ptr, std::bind and
std::mem_fn - they are already in the draft standard library
extension.

Tom

C++ FAQ: http://www.parashift.com/c++-faq-lite/
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html

 

[David Harmon]

could write a class (called eg. Buffer) to encapsule your char buffers and
use auto_ptr<Buffer> in your code then.

std::auto_ptr<> cannot be used for this purpose because it uses "delete"
of the owned pointer, and not "delete[]"

 

[lallous]

shared_ptr<FILE> fp1(fopen("Filename", "r+"), fclose);



Use some kind of scope guard class. boost::shared_ptr is good for most
uses (particularly when combined with boost::bind and boost::mem_fn).
You might prefer an alternative such as:

http://www.cuj.com/documents/s=8000/cujcexp1812alexandr/

Note that there should soon be std::shared_ptr, std::bind and
std::mem_fn - they are already in the draft standard library
extension.

Tom

C++ FAQ: http://www.parashift.com/c++-faq-lite/
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html

Thanks to all who shared their comments.

I liked most the vector<char> instead of allocating a char * w/ new char[].
Also liked the shared_ptr<FILE> ... .

I am still new to C++ and was expecting some solution that doesn't involve
objects as the STD library and other objects are loaded with lots of code
and checking which makes the code a bit slower.

Will ask this again in comp.lang.c to get yet more approaches.

 

[tom_usenet]

Thanks to all who shared their comments.

I liked most the vector<char> instead of allocating a char * w/ new char[].
Also liked the shared_ptr<FILE> ... .

I am still new to C++ and was expecting some solution that doesn't involve
objects as the STD library and other objects are loaded with lots of code
and checking which makes the code a bit slower.

Code using standard libraries tends to be faster than code that
doesn't. Matching the optimization effort your library implementor has
gone to for the general case container can be hard even for a specific
case. In any case, the work is generally not worth it - improving
algorithms typically gives you more of a speed up.

Tom

C++ FAQ: http://www.parashift.com/c++-faq-lite/
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html

 

[Karl Heinz Buchegger]

I liked most the vector<char> instead of allocating a char * w/ new char[].
Also liked the shared_ptr<FILE> ... .

I am still new to C++ and was expecting some solution that doesn't involve
objects as the STD library

Then you are reinventing the wheel for no good reason.

and other objects are loaded with lots of code
and checking which makes the code a bit slower.

I bet that you will not be able to beat the performance of std::vector
(If used right) with homegrown code by more then 5%.

 

[Old Wolf]

void fnc() {

char *mem1, *mem2, *mem3, *mem4;
// lots of code...
// mem1, 2, 3, 4 got allocated
// lots of code and condition checks
if (condition_failed)
// (etc.)

Assuming you don't like the suggestions offered so far (ie. create
a class where your cleanup code runs in the destructor), you
could do this:

void fnc() {
char *mem1 = 0, *mem2 = 0, *mem3 = 0, *mem4 = 0;
try {
// lots of code
if (condition1_failed) {
// condition1-specific cleanup code
}
}
delete mem1; delete mem2; delete mem3; delete mem4;
}

There is actually no need for __finally and __leave, you can
always structure your function so that "try" and "catch" are
sufficient.

 

[puppet_sock]

Any better way, other than goto or compiler specific solution?

Just in case nobody explicitly stated the rule to apply here.

Resource allocation should usually happen in a class.
Resource de-allocation should usually happen in the dtor of a class.

This applies not just to memory, but files, and anything else
that has to be allocated, gotten, initialized, etc. So when
you get into things like networking, for example, and you
have to open a connection, that should be done in a class and
given back in the dtor.

Then you create an instance of a class and have that object do
the allocation. When the object goes out of scope, the dealloc
hapens automatically. This will work properly wether it happens
because you returned normally form the function, or because you
trapped an error and aborted, or because some sub-component
threw an exception.

The advantages of this are many. For example, you can now start
working on making your code exception safe. You can have your
functions throw exceptions and catch them with some assurance
that you won't have resource leaks. And you won't have to struggle
to remember all the stuff you allocated, and in what order,
so you can deallocate it on error conditions.

This is why so many folks were suggesting you use the std string
objects. (Well, one reason.)
Socks

 

[Jeff Flinn]

shared_ptr<FILE> fp1(fopen("Filename", "r+"), fclose);



Use some kind of scope guard class. boost::shared_ptr is good for most
uses (particularly when combined with boost::bind and boost::mem_fn).
You might prefer an alternative such as:

http://www.cuj.com/documents/s=8000/cujcexp1812alexandr/

Note that there should soon be std::shared_ptr, std::bind and
std::mem_fn - they are already in the draft standard library
extension.

Tom

C++ FAQ: http://www.parashift.com/c++-faq-lite/
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html

Thanks to all who shared their comments.

I liked most the vector<char> instead of allocating a char * w/ new char[].
Also liked the shared_ptr<FILE> ... .

I am still new to C++ and was expecting some solution that doesn't involve
objects as the STD library and other objects are loaded with lots of code
and checking which makes the code a bit slower.

"lots of <source> code" != "lots of machine instructions" - atleast in C++ -
release/optimized builds with any good compiler will only generate the code
that's actually used.

Will ask this again in comp.lang.c to get yet more approaches.

Sounds like this was your predisposition anyway.

Jeff F