5

I need to deal with two struct addrinfo pointers. Since I'm coding in C++(11), I've to make my code exception-safe. Indeed, my costructors may throw a runtime_error. When you don't need that kind of struct anymore, you should call freeaddrinfo in order to free the list inside the struct. Please consider the following code:

#include <memory>
#include <netdb.h>

class SomeOtherClass
{
  public:
    SomeOtherClass() : hints(new addrinfo), result(new addrinfo) { /*stuff*/ }
    ~SomeOtherClass() { freeaddrinfo(result.get()); } // bad things will happen

private:
    std::unique_ptr<addrinfo> hints, result;
};

class MyClass : public SomeOtherClass
{
public:
    MyClass() { /* hints initialization, call to getaddrinfo, etc. */ }

private:
    // ...
};

My questions are:

  1. addrinfo is an "old" C structure, with no ctor/dtor to call: is it safe to use new?
  2. getaddrinfo requires a pointer to a pointer to a addrinfo struct: how should I pass it via smart pointers?
  3. What about calling freeaddrinfo? It's considered unsafe to delete (or better free) the pointer that the smart pointer is holding.

For hints there is no problem, since its lifetime is smaller.

edmz
  • 8,220
  • 2
  • 26
  • 45
  • Well you don't really allocate the resulting address info structures yourself, do you? They are allocated by the `getaddrinfo` function. And you can set a custom deleter for smart pointers, which can call `freeaddrinfo`. And the hints doesn't have to be allocated dynamically, not even if you're going to use the same structure multiple times. Just use the address-of operator on a normal (non-pointer) structure variable. – Some programmer dude Feb 21 '14 at 18:28
  • 1
    This constructor is [not safe](http://herbsutter.com/gotw/_102/) you should use `std::make_unique` if your compiler supports it, or make your own equivalent if it doesn't. You really really don't want to deal with the memory leaks from partial construction. Also if your base class is going to hold resources your base class destructor should be `virtual` – Mgetz Feb 21 '14 at 18:30
  • @Mgetz that's just a snippet to make you figure out the problem. However, I can't find a place for a `virtual` dtor, because the base class is not an abstract one. – edmz Feb 21 '14 at 18:42
  • @black declaring a destructor `virtual` does not mean pure virtual, just prefixed with the `virtual` keyword, this is so that if someone holds onto a base class pointer both the base class and the derived class destructors are called – Mgetz Feb 21 '14 at 18:43
  • @Mgetz this is safe for ctors as the members are initialized in the order they are declared and can be used by the remaining ones – galop1n Feb 21 '14 at 19:03
  • @Mgetz A better way would be firstly `nullptr`ing `result` and then carefully try to `new` (maybe in a `try-catch` block). – edmz Feb 21 '14 at 19:15
  • Even using directly `shared_ptr` could be a solution. Anyways, I'm still stuck on the second point, can you check it out? – edmz Feb 21 '14 at 19:19
  • @Mgetz members already construct have their dtor called normally http://coliru.stacked-crooked.com/a/af35b86b4ab61173 ! "#include struct A { A() { std::cout << "A" << std::endl; } ~A() { std::cout << "~A" << std::endl; } }; struct B { B() { throw 1; } }; struct C { A a_; B b_; }; int main() { try { C c; } catch (...){ } }" – galop1n Feb 21 '14 at 19:21
  • @galop1n I appreciate the debate, but please do not go off-topic. – edmz Feb 21 '14 at 19:24
  • @Mgetz the 4th parameter must be a pointer to a pointer to `addrinfo`. I cannot allocate in statically. – edmz Feb 21 '14 at 19:26
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/48101/discussion-between-galop1n-and-black) – galop1n Feb 21 '14 at 19:30

2 Answers2

9

For any addrinfo you allocate yourself, it is safe to use newand delete, so you can use the default implementation of unique_ptr to handle that.

For any addrinfo that getaddrinfo() allocates, you must use freeaddrinfo() to free it. You can still use unique_ptr for that, but you must specify freeaddrinfo() as a custom Deleter, eg:

class SomeOtherClass
{
  public:
    SomeOtherClass() : hints(new addrinfo), result(nullptr, &freeaddrinfo) { /*stuff*/ }

private:
    std::unique_ptr<addrinfo> hints;
    std::unique_ptr<addrinfo, void(__stdcall*)(addrinfo*)> result;
};

Then you can do this:

getaddrinfo(..., &result);

Or this, if std::unique_ptr does not override the & operator:

addrinfo *temp;
getaddrinfo(..., &temp);
result.reset(temp);

UPDATE: a better option is to use decltype and let the compiler deduce the function type of the Deleter for you:

std::unique_ptr<addrinfo, decltype(&freeaddrinfo)> result;
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • This is not safe and risks partial construction please see the link in my comment on the OP. – Mgetz Feb 21 '14 at 18:37
  • AFAIK `make_unique` does not allow you to specify a custom `Deleter`, though. – Remy Lebeau Feb 21 '14 at 18:47
  • 3
    Personally, I would not allocate the `hints` on the heap to begin with, so I would just make it a local member of the class without dynamic allocation at all. Then there is no chance of partial construction (in this example, anyway). You don't have a choice in how the `result` is allocated. – Remy Lebeau Feb 21 '14 at 18:53
  • @RemyLebeau it appears you're right... in that case the only exception safe alternative would be to delegate to a default constructor to get the object out of the construction phase so the destructor will be called. – Mgetz Feb 21 '14 at 18:58
  • What do you mean with "You don't have a choice in how the result is allocated"? – edmz Feb 21 '14 at 18:59
  • BTW, simply passing `&result` does not work as well as `result.get()`. – edmz Feb 21 '14 at 19:05
  • You may even use the non static member initialisation syntax, would be clearer and factorize code if there is more than one ctor – galop1n Feb 21 '14 at 19:27
  • @black: `getaddrinfo()` allocates the output `addrinfo`. You have no control over how that memory is allocated. That is why there is a `freeaddrinfo()` function to free that memory. – Remy Lebeau Feb 21 '14 at 20:43
  • @black: As for using `&result`, that assumes `std::unique_ptr` overrides the `&` operator like `std::auto_ptr` does. In case it does not, that is why I also showed the `reset()` alternative. `result.get()` will not work because it returns the current `addrinfo*` pointer that `unique_ptr` is holding (which is initially null), but `getaddrinfo()` is expecting a `addrinfo**` (which an overridden `&` operator would return), ie an address to an `addrinfo*` variable that `getaddrinfo()` can modify to point at the `addrinfo` it allocates. – Remy Lebeau Feb 21 '14 at 20:44
1

In addition to other answers: C++23 introduces a new feature for smart pointers interacting with C-style output parameters. Now, instead of replacing the managed object with a temporary value, we can use std::out_ptr():

getaddrinfo(..., std::out_ptr(result));
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
Doliman100
  • 81
  • 1
  • 2
  • 4