5

I am using scoped_ptr inside small functions like this. so that I don't have to call delete. Is this an overkill for this usage? My team members prefer raw pointers and delete. What is the cost of using scoped_ptr if this happens to be used in a very critical path? Shouldn't this be in-lined and be exactly equivalent to just using normal delete in optimized binary?

void myfunc()
{
  boost::scoped_ptr<myobj> objptr = someFactory::allocate();
  callsomeotherfunc(objptr.get());
}
balki
  • 26,394
  • 30
  • 105
  • 151

4 Answers4

8

I am unsure of the performance hit, but using scoped_ptr here ensures myfunc() is exception safe: if callsomeotherfunc() throws an exception the dynamically allocated memory will still be freed. If scoped_ptr was not used and callsomeotherfunc() could throw then the function would have to be structured similar to this:

void myfunc()
{
    myobj* objptr = someFactory::allocate();

    try
    {
        callsomeotherfunc(objptr);
        delete objptr;
    }
    catch (const some_exception&)
    {
        delete objptr;
        throw;
    }
}

This is error prone as all future modifications of the function would need to ensure that delete objptr; is invoked on all possible exit points.

hmjd
  • 120,187
  • 20
  • 207
  • 252
  • 1
    I expect `boost::scoped_ptr` to be as fast in the non-exceptional case and much faster in the exceptional one (throwing is costly with the modern ZeroCost model). – Matthieu M. Apr 27 '12 at 13:08
3

I would not use scoped_ptr for that purpose, but rather unique_ptr in C++11 and auto_ptr in older compilers, both of which are equivalent for your particular use case. As of whether it is overkill, no, it is not, it is the only option to providing exception safety (say anything in the code of either myfunc or callsomeotherfunc throws, you want the memory to be released). Performance wise, the three options are equivalent to having a delete call at the end of the function in the case where exceptions are not thrown, and faster than having a try/catch block with a delete that rethrows the exception in the event of exceptions.

Additionally, it seems that you are allocating from a factory, in some designs that factory will have a deallocate operation that needs to be called, rather than delete. If that is the case, you might consider a different smart pointer (shared_ptr from the standard, which would be overkill if delete is the proper way of releasing the memory; or some short of managed_ptr for which you can also provide a deleter)

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • what benefit does `auto_ptr` give over `scoped_ptr` (aside from one being `std` and one being `boost`)? – KillianDS Apr 27 '12 at 12:58
  • @KillianDS: For this particular case none. But it is standard and does not bring an extra external dependency, and in this case it is a valid solution. – David Rodríguez - dribeas Apr 27 '12 at 13:00
  • 1
    @DavidRodríguez-dribeas: the semantics of `boost::scoped_ptr` are quite different from that of `std::unique_ptr`. A notable property of `scoped_ptr` is to add a semantic meaning for the benefit of the reader: this resource will not escape this scope. – Matthieu M. Apr 27 '12 at 13:07
0

No, it is not. The benefit of using smart pointer (e.g. exception safety and automatic resource cleanup) is far higher than performance loss of using couple of additional bytes of memory and couple of additional CPU clocks needed for smart pointer creation, maintenance and destruction.

Bojan Komazec
  • 9,216
  • 2
  • 41
  • 51
  • ..except if the function reaches a point when exception safety and automatic resource cleanup must absolutely not happen. I'm guessing that setting the smart pointer to null would prevent the cleanup? – Martin James Apr 27 '12 at 13:14
  • @Martin That's interesting idea but I cannot think of any case when exception safety and automatic resource cleanup are undesirable - could you please provide some example? – Bojan Komazec Apr 27 '12 at 13:53
  • Sure, easy: Rename 'callsomeotherfunc(objptr);' to 'queueObjectToOutputThread(objptr);'. Now, automatic resource cleanup will cause a disastrous AV/segfault, probably in some place in another module, run by another thread, unrelated to the supposedly 'safe' module that caused the problem. – Martin James Apr 27 '12 at 16:01
  • @MartinJames Yeah but that relates to misuse of smart pointers and the question about the choice on pointer type. It depends on whether `callsomeotherfunc(myobj* p)` takes the ownership over the object or not. If it does, passing `p` (raw pointer, not wrapped in any kind of smart pointer) is the only safe way. If it doesn't, it is fine to use `boost::scoped_ptr` and pass pointer it wraps. OP considers `callsomeotherfunc` not taking ownership and he is asking whether to use `delete` at the end of scope or to use smart pointer (`My team members prefer raw pointers and delete.`). – Bojan Komazec Apr 30 '12 at 11:36
  • @MartinJames It would be a bad design if `callsomeotherfunc` in the example above takes the ownership - you pointed out what would happen if raw pointer owned by smart pointer is passed to it. – Bojan Komazec Apr 30 '12 at 11:44
0

I think that the equivalent to scoped_ptr usage would be:

void myfunc()
{
  myobj* objptr = someFactory::allocate();
  try
  {
      callsomeotherfunc(objptr);
  }
  catch (...)
  {
      delete objptr;
      throw;
  }
  delete objptr;
}

I know which version I'd prefer to write...

Adrian Conlon
  • 3,941
  • 1
  • 21
  • 17