List:MySQL++« Previous MessageNext Message »
From:Warren Young Date:November 29 2007 8:05am
Subject:Re: Eyeballs needed on new reference counted pointer template
View as plain text  
Jonathan Wakely wrote:
> Hi Warren, here's a modified RefCountedPointer, with the following changes:

Mostly applied as-is, with some changes detailed below.

You've now made it into the CREDITS file.  :)

> * Added a no-throwing swap operation. 

Is there a good reason for this to be public?  Isn't it just an 
implementation detail for assign()?

> (Should probably define a global
> swap(x, y) to use this member)

Why?

> * Safe bool idiom replaces operator bool and operator!

I was going to do this, but just yesterday I found the need in 
RefCountedPointer for operator== and operator!= as well.  I can get that 
and also implicit conversion to bool-like with operator void*.  The safe 
bool article warns against it in part because it lets you say:

	RefCountedPointer<Foo> bar(new Foo);
	delete bar;

But *really*.  If one is so intent on shooting oneself in the foot....

The only time this should happen accidentally is when converting an 
unmanaged pointer to a refcounted pointer and forgetting to remove the 
deletes you previously needed.  Such a bug will show up the first time 
you run the program as it results in a double-delete.  I've documented 
the risk in the refman and am content to leave it at that.

The other warnings in the article about operator void* don't really 
apply here.  RefCountedPointer _is_ a pointer after all.  It's perfectly 
legitimate that you should be able to convert it to void*, unlike in the 
example he gives where cin/cout have no pointerness to them.

Your safe-bool code may still make its way into MySQL++, because there 
are other operator bools on classes that, like cin/cout also have no 
pointerness to them.

> ...safe against self-assignment...

I don't see any checks for self-assignment in your code.  It sort of 
happens implicitly in assign(const ThisType&) because the refcount just 
goes up and then comes right back down, but this is wasteful.  And in 
the assign(T*) case, I'm pretty sure assigning the same pointer will 
result in a double-delete.  I've added explicit checks to both assign() 
overloads.
Thread
Re: Eyeballs needed on new reference counted pointer templateWarren Young29 Nov
  • Re: Eyeballs needed on new reference counted pointer templateJoseph Artsimovich29 Nov
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young29 Nov
      • Re: Eyeballs needed on new reference counted pointer templateJonathan Wakely30 Nov
        • Re: Eyeballs needed on new reference counted pointer templateWarren Young1 Dec
  • Re: Eyeballs needed on new reference counted pointer templateJonathan Wakely30 Nov
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young1 Dec
  • Re: Eyeballs needed on new reference counted pointer templateJonathan Wakely30 Nov
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young1 Dec