How to free up pointers

[Doug Haber]

Hi All,

I'm a c++ newbie and I was hoping you could help me out with what I think is
a common problem (actually a few problems.). I want to get a pointer that I
can pass around to do some stuff, so I'm trying to write a function that
looks like:

HRESULT getThingy(Thingy* thingy)

Question 1: I assume the common way to do this is to return an HRESULT and
pass in a pointer as opposed to creating a class, or the java way, which
would look like:

Thingy getThingy()

Question 2: I use a bunch of pointers in my function, and I know I need to
clean them up but due to the structure of the function I'm not sure what's
the clean way to do it. Is there a typical approach? The function looks
something like

HRESULT getThingy(Thingy* thingy) {
A* a;
B* b;

if (SUCCESS(getA(a))){
if (a->doSomething()){
if (SUCCESS(a->getB(b))){
if (!b->foo()){
b->release();
a->release();
return E_FAIL;
}
if(SUCCESS(b->getThingy(thingy))){
b->release();
a->release();
return S_OK;
}
}
}
}
b->release();
a->release();
return E_FAIL;
}


The b->release() and a->release() all over the place seems pretty nasty.
Is there a standard way to handle this kind of issue?

Thanks!
-Doug

 

[ironfelix]

The common solution is: have a varible result, one return only from
the function - at the end of the function and universal label out with
many gotos to it. Code would good to read and maintain.

return_t function(arg_t arg){
return_t result;
local_objects *a=NULL,*b=NULL;
....................
if( something( freaked up) ) {
result= BAD;
goto out;
}
.............................................
if( something( very cool)){
result= OK;
goto out;
}
.................................
out:
if( a) release(a);
if( b) release(b);
return result;
}

Doug Haber пиÑал(а):

 

[Nate Barney]

The common solution is: have a varible result, one return only from
the function - at the end of the function and universal label out with
many gotos to it. Code would good to read and maintain.

This, however, is not necessarily exception-safe, and it uses goto,
which many consider to be distasteful. It would be better to use the
Resource Acquisition Is Initialization (RAII) idiom. See
http://en.wikipedia.org/wiki/Resource_acquisition_is_initialization

Also, this is not a flame, but please don't top-post your replies. In
this group, it's generally frowned upon.

Nate

 

[David Harmon]

On Mon, 11 Sep 2006 23:31:29 -0400 in comp.lang.c++, "Doug Haber"

The b->release() and a->release() all over the place seems pretty nasty.
Is there a standard way to handle this kind of issue?

Your HRESULT type and release() must come from somewhere other than
the standard C++ language or library, so you may have some specific
needs that go beyond the topic of this newsgroup.

The most obvious case in C++ is objects allocated with new that have
to be deleted. std::auto_ptr<> takes care of that in the simplest
possible case, calling delete automatically when it goes out of
scope. As far as I can see from your example, you could get rid of
all those release() calls with a similar smart pointer class that
calls release() when it is destructed.

If you need something a little more flexible, there are a variety of
customizable smart pointer classes available, some of which might be
helpful. I'd look at std::tr1::shared_ptr or boost::shared_ptr
first (http://boost.org)

 

[Michael]

I want to get a pointer that I
can pass around to do some stuff, so I'm trying to write a function that
looks like:

HRESULT getThingy(Thingy* thingy)

Question 1: I assume the common way to do this is to return an HRESULT and
pass in a pointer as opposed to creating a class, or the java way, which
would look like:

Thingy getThingy()

I would do the latter. In plain old C, you had to do the whole HRESULT
thing, but C++ has exception handling, so I tend to do your second and
throw an exception if something goofy happens (but I too have been
influenced by Java).

Question 2: I use a bunch of pointers in my function, and I know I need to
clean them up but due to the structure of the function I'm not sure what's
the clean way to do it. Is there a typical approach? The function looks
something like

HRESULT getThingy(Thingy* thingy) {
A* a;
B* b;

if (SUCCESS(getA(a))){
if (a->doSomething()){
if (SUCCESS(a->getB(b))){
if (!b->foo()){
b->release();
a->release();
return E_FAIL;
}
if(SUCCESS(b->getThingy(thingy))){
b->release();
a->release();
return S_OK;
}
}
}
}
b->release();
a->release();
return E_FAIL;
}


The b->release() and a->release() all over the place seems pretty nasty.
Is there a standard way to handle this kind of issue?

I respectfully disagree with the prior post with gotos. I used to
write code like that in C, and that really was the best, cleanest way
to do it. But C++ has a better mechanism, using the auto_ptr class in
the standard library.

Your code would look something like this:

#include <memory>

using std::auto_ptr;

A* getA() {
// Some code that returns an A*, or throws an exception on failure
}

B* getB() {
// Some code that returns a B*, or throws an exception on failure
}

Thingy* getThingy() {
auto_ptr<A> a(getA());
if (a->doSomething()) {
auto_ptr<B> b(getB());
if (!b->foo) {
throw something;
}
return b->getThingy();
}
throw something;
}

In fact, this kind of thing (using a class that automatically frees
resources when the object goes out of scope) is so common in C++ that
it has its own name: Resource Acquisition Is Initialization (RAII).

Michael

 

[benben]

HRESULT getThingy(Thingy* thingy) {

A* a;
B* b;

if (SUCCESS(getA(a))){
if (a->doSomething()){
if (SUCCESS(a->getB(b))){
if (!b->foo()){
b->release();
a->release();
return E_FAIL;
}
if(SUCCESS(b->getThingy(thingy))){
b->release();
a->release();
return S_OK;
}
}
}
}
b->release();
a->release();
return E_FAIL;
}


The b->release() and a->release() all over the place seems pretty nasty.
Is there a standard way to handle this kind of issue?

Just a (slightly off-topic) note for the OP: your code looks horribly
like manipulating Microsoft COM pointers. If that is the case, you have
to disregard all the previous replies so far because using smart
pointers that calls the object's destructor will be disastrous.

Regards,
Ben

 

[David Harmon]

On Tue, 12 Sep 2006 18:07:33 +1000 in comp.lang.c++, benben

Just a (slightly off-topic) note for the OP: your code looks horribly
like manipulating Microsoft COM pointers. If that is the case, you have
to disregard all the previous replies so far because using smart
pointers that calls the object's destructor will be disastrous.

Not at all. There's more than one kind of smart pointer. You may
have to roll your own, as in "a similar smart pointer class that
calls release() when it is destructed". shared_ptr can be given a
custom deallocater function. etc.

 

[tragomaskhalos]

Just a (slightly off-topic) note for the OP: your code looks horribly
like manipulating Microsoft COM pointers. If that is the case, you have
to disregard all the previous replies so far because using smart
pointers that calls the object's destructor will be disastrous.

This is the crux of it. If, as seems likely, the OP is working with COM
then he has a choice of using the ugly HRESULT style of working, or of
using MS-supplied wrapper classes that map HRESULT errors to
exceptions. The latter is not a panacea however, if you're working with
COM and C++ then you absolutely have to understand IUnknown,
QueryInterface and reference counting even if these things are wrapped.
There's quite a lot to this, I warn you now. If the OP's code is not
COM but just (for some reason) uses a COM style, then I'd look at
rewriting it along the lines already suggested by others.

 

[Doug Haber]

Hi All,

Thanks so much for the flurry of responses. I am in fact using COM
(excellent deduction), so does that mean I'm supposed to use a try...catch
and throw exceptions instead of returnting HRESULTs? Also, can you be more
specific on what I should learn about IUknown, QueryInterface and reference
counting?

Thanks so much!
-Doug

 

[tragomaskhalos]

Hi All,

Thanks so much for the flurry of responses. I am in fact using COM
(excellent deduction), so does that mean I'm supposed to use a try...catch
and throw exceptions instead of returnting HRESULTs? Also, can you be more
specific on what I should learn about IUknown, QueryInterface and reference
counting?

On your first question, it's a matter of taste. Using raw HRESULTs,
AddRefs and all that is ugly but it has the advantage of being explicit
and not hiding what's going on under the bonnet. Alternatively Visual
C++ does provide a smart pointer implementation that you can use to
hide a lot of this stuff and translate HRESULT errors into exceptions
(it's been 5 or 6 years since I did this so the details are fuzzy, but
code gets generated that you can use when you import a dll or type
library). Do *not* attempt to write your own smart pointer for this
btw.

I'm a firm believer in not using an abstraction layer until you
understand what's being abstracted, because all abstractions leak to
some extent and when that happens you won't have a clue what's really
going on.

As for IUnknown et al, google or MSDN is your friend. However be warned
that COM is Microsoft's unwanted stepchild, .NET is the blue-eyed boy,
even though a huge amount of their stuff is still COM underneath the
covers/hype. Alternatively Don Box's book "Essential COM" is the
definitive text.

Finally this is all now definitely off topic (apologies to the group),
a suitable MS list would be able to provide further help.

HTH