On 11/2/07, Mike Tegtmeyer <tegtmeye@eecis.udel.edu> wrote:

I am often asked why shared_ptr always uses the custom deleter even when the pointer argument is zero. I know it is safe to delete a NULL pointer so this works with delete but it would seem that if the user is giving a custom deleter, then they are probably doing something else. ie

//will crash if fread returns 0 shared_ptr<FILE> foo(fread("foo.txt","r"),fclose);

This is an easy screw-up for most new folks and the docs don't really explicitly point out this (I think) non-obvious behavior.

One can obviously wrap the deleter with a check among numerous other things but I'm curious what the rationale was since this won't work "out of the box" with C APIs. Can someone point me to the dialog discussing the design decision for this behavior so I can pass it along?

I don't think shared_ptr is designed to work on things like a FILE*. One uses filestream or writes a simple class wrapping up the FILE*.

Mike Tegtmeyer:

I am often asked why shared_ptr always uses the custom deleter even when the pointer argument is zero. I know it is safe to delete a NULL pointer so this works with delete but it would seem that if the user is giving a custom deleter, then they are probably doing something else. ie

//will crash if fread returns 0 shared_ptr<FILE> foo(fread("foo.txt","r"),fclose);

This is a good question without an easy answer. There is a conflict between semantical purity (shared_ptr does not attempt to interpret the pointer value in any way, this task is left to the deleter and ultimately to the programmer) and expressive power (the user may, in fact, want his deleter to be called even for NULL) on one side, and convenience for use cases like fopen, on the other. Neither alternative is a clear winner. What tips the scales somewhat is that (a) typically an idiomatic C++ wrapper over FILE would not construct a shared_ptr when fopen fails, it will throw an exception, (b) many resource releasing functions (such as 'free') do ignore NULLs instead of crashing, and (c) there is a relatively easy (if inconvenient) workaround. (If shared_ptr didn't call the deleter for NULLs, one would have to use somewhat awkward tricks such as encode NULL as (void*)-1 in the reverse case.)

Good answer The common retort from new folks that I hear to the "no attempt to interpret the pointer value" argument is more of a pragmatic one: it doesn't behave as expected in the fundamental case. I appreciate the other side of the coin awkward trick example however, and as I sit and think about it - that is much more painful. The counter argument that I hear from not-new folks however is that it takes some work to not leak the resource during an exception in the shared_ptr construction. Therefore, the ugliness to maintain integrity diminishes it's usefulness. ie FILE *in = fopen(...); if(!in) return/throw badness; //may throw here, 'in' is lost shared_ptr<FILE> fin(in,fclose); Therefore, <third_person> struct file_sentry { file_sentry(FILE *_f) :f(_f) {} ~file_sentry() { if(f) fclose(f); } FILE *f; }; file_sentry sentry(fopen(...)); if(!sentry.f) return/throw badness; //if throw here, 'in' gets closed shared_ptr<FILE> fin(sentry.f,fclose); sentry.f = 0; But gee, that was a lot of work, maybe if I just refactor file_sentry to resource_sentry so it does what I want... - or - shared_ptr<file_sentry> ... do I really have to call new here??? - will maybe if I point shared_pointer to a local object, eh my head hurts, never mind, exception safety is just mental gymnastics anyway... </third_person> I can understand the frustration. Mike On Sat, 3 Nov 2007, Peter Dimov wrote:

Mike Tegtmeyer:

...

I am often asked why shared_ptr always uses the custom deleter even when the pointer argument is zero. I know it is safe to delete a NULL pointer so this works with delete but it would seem that if the user is giving a custom deleter, then they are probably doing something else. ie

//will crash if fread returns 0 shared_ptr<FILE> foo(fread("foo.txt","r"),fclose);

This is a good question without an easy answer. There is a conflict between semantical purity (shared_ptr does not attempt to interpret the pointer value in any way, this task is left to the deleter and ultimately to the programmer) and expressive power (the user may, in fact, want his deleter to be called even for NULL) on one side, and convenience for use cases like fopen, on the other.

Neither alternative is a clear winner. What tips the scales somewhat is that (a) typically an idiomatic C++ wrapper over FILE would not construct a shared_ptr when fopen fails, it will throw an exception, (b) many resource releasing functions (such as 'free') do ignore NULLs instead of crashing, and (c) there is a relatively easy (if inconvenient) workaround.

(If shared_ptr didn't call the deleter for NULLs, one would have to use somewhat awkward tricks such as encode NULL as (void*)-1 in the reverse case.)

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Ah! That was news to me. I have the rationale that I needed. Thanks. Mike On Sat, 3 Nov 2007, Peter Dimov wrote:

Mike Tegtmeyer:

...

The counter argument that I hear from not-new folks however is that it takes some work to not leak the resource during an exception in the shared_ptr construction. Therefore, the ugliness to maintain integrity diminishes it's usefulness. ie

FILE *in = fopen(...); if(!in) return/throw badness;

//may throw here, 'in' is lost shared_ptr<FILE> fin(in,fclose);

The answer here is much, much easier. 'in' will not be lost. shared_ptr does invoke the deleter when the constructor throws.

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

I don't think making file_entry is a lot of work. It's pretty simple and straightforward. And once it's made, you keep it around in your arsenal.

Not to split hairs here, but you could also write: boost::shared_ptr<FILE> open_file( char const * name, char const * mode ) { if( FILE * f = fopen(name,mode) ) return boost::shared_ptr<FILE>(f,fclose); else throw fopen_error(name,mode); } and keep that in your "arsenal". In fact, I think this function would be a good addition to boost so it's part of everyone's arsenal. :) -- Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

I agree but it quickly becomes a pain because you end up writing a wrapper for every thing of this type to satisfy this 'wart' with shared_ptr: pthread_create ... opendir fopen open free COM objects the list goes on. I guess if shared_ptr was to be done all over again, I think that I would argue that it should act like a pointer, ie you shouldn't be able to 'delete' a 'null' pointer - whatever delete and null mean for that type that is a pointer concept. Actually I think that describes it best; shared_ptr should expect a pointer concept and a pointer concept shouldn't define the notion of deletion for invalid values as a precondition. Thus allowing it to work out of the box will tons of legacy and C API''s. Again, if it had to be done all over, I'd assert what we have now is better described as a shared_resource (something that doesn't interpret its type). My .02, but what is done is done I guess. Mike On Nov 3, 2007, at 3:30 AM, Emil Dotchevski wrote:

...

I don't think making file_entry is a lot of work. It's pretty simple and straightforward. And once it's made, you keep it around in your arsenal.

Not to split hairs here, but you could also write:

boost::shared_ptr<FILE> open_file( char const * name, char const * mode ) { if( FILE * f = fopen(name,mode) ) return boost::shared_ptr<FILE>(f,fclose); else throw fopen_error(name,mode); }

and keep that in your "arsenal".

In fact, I think this function would be a good addition to boost so it's part of everyone's arsenal. :)

-- Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/ listinfo.cgi/boost

You lost me there.

It's perfectly fine to delete a null pointer.

5.3.5/2 says

"if the value of the operand of delete is the null pointer the operation has no effect."

I am using delete in the general sense ie fclose, closedir, etc. And even though this doesn't directly map here, this also holds true for resources that been obtained with allocators; it is not perfectly fine to deallocate them if they are null(20.1.5) even though I would assert that a handle to resources obtained with std::allocator::allocate conform to a pointer concept. Mike

Sure. I wasn't really arguing changing shared_ptr, nor did I think that there was a number of different ways to work around the problem (it was good to see the number of different ways folks would though). I was more making a point about semantic design and the fact that if one looked at all of the things that follow this paradigm, obtain some resource with a free function returning a pointer and release the resource with another free function that takes a pointer, I'd argue that more things do not allow you to release or 'delete' a null pointer than do (obviously new are free are ones that do). That's all. Mike On Nov 4, 2007, at 3:00 AM, Daniel Frey wrote:

On Sat, 2007-11-03 at 19:45 -0400, Mike Tegtmeyer wrote:

...

I guess if shared_ptr was to be done all over again, I think that I would argue that it should act like a pointer, ie you shouldn't be able to 'delete' a 'null' pointer - whatever delete and null mean for

Let's keep shared_ptr like it is. What you (and others) probably need is the right helper, as a quick hack it looks like this:

#include <iostream> #include <boost/shared_ptr.hpp>

struct X {};

void freeX( X* x ) { // Should not be called with a null pointer std::cout << "free " << x << std::endl; }

template< typename T > struct on_nonzero_t { typedef void (&F)( T* ); F f_;

on_nonzero_t( F f ) : f_( f ) {} void operator()( T* t ) const { if( t ) f_( t ); } };

template< typename T > on_nonzero_t< T > on_nonzero( void (&f)( T* ) ) { return on_nonzero_t< T >( f ); }

void f( X* x ) { std::cout << "x = " << x << std::endl; boost::shared_ptr< X > tmp( x, on_nonzero( freeX ) ); }

int main() { X x; f( &x );

f( 0 ); }

The naming of the helper might be crap, but anyway, hope you can make some use of it...

Regards, Daniel

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/ listinfo.cgi/boost

We should be able to solve this ;) We have: template<class Y, class D> shared_ptr <http://www.boost.org/libs/smart_ptr/shared_ptr.htm#constructors>(Y * p, D d); So, for type 'Y', we can have the default deleter default_delete<Y*>(); template<class Y> struct default_delete { void do_delete(Y p) { delete p; } }; template<> struct default_delete<FILE*> { void do_delete(FILE* p) { if (p) fclose(p); } }; ...etc Users can even specialize default_delete for their custom classes if they wish. Best, John

I agree but it quickly becomes a pain because you end up writing a wrapper for every thing of this type to satisfy this 'wart' with shared_ptr:

pthread_create ... opendir fopen open free COM objects

the list goes on.

-- http://John.Torjo.com -- C++ expert ... call me only if you want things done right

On Sunday 04 November 2007 00:45:47 Mike Tegtmeyer wrote:

I agree but it quickly becomes a pain because you end up writing a wrapper for every thing of this type to satisfy this 'wart' with shared_ptr:

pthread_create ... opendir fopen open free COM objects

the list goes on.

I'd take that even further, not all such resources are represented as pointer (think filedescriptors or win32 HANDLEs), so shared_ptr doesn't help at all. Further, you usually only need a scope-based lifetime, not true sharing with reference counting, so std::auto_ptr or boost::scoped_ptr should be enough. Lastly, you don't need a smart pointer at all, because the overloads of -> and * are typically never used with those opaque handles. However, sometimes things like returning them from functions is used, but in those cases the behaviour of std::auto_ptr (i.e. ownership transfer) is fully sufficient. I posted a wrapper that gives you just that some time ago here, asking for comments. I didn't get any, probably because it was in the hot phase of the last release. I'll post the whole mail again, the subject is "helper for managing legacy resources". Uli

I think the obvious rationale is that while both approaches have their merits, you can indeed implement the one based on the other but not vice versa. I agree. I have worked with various APIs which use integer types as handles and use -1 as the null state. If shared_ptr automatically ignored handles that compare equal to 0, it wouldn't be usable with these APIs. I also don't think that it's unreasonable, or even necessary to justify, that custom deleters should behave like delete/free.