63

Think to a C function that return something that must be freed, for example the POSIX's strdup(). I want to use that function in C++11 and avoid any chance of leaks, is this a correct way?

#include <memory>
#include <iostream>
#include <string.h>

int main() {
    char const* t { "Hi stackoverflow!" };
    std::unique_ptr<char, void(*)(void*)>
        t_copy { strdup(t), std::free };

    std::cout << t_copy.get() << " <- this is the copy!" <<std::endl;
}

Assuming it makes sense, it is possible to use a similar pattern with non-pointers? For example for the POSIX's function open that returns an int?

Paolo.Bolzoni
  • 2,416
  • 1
  • 18
  • 29
  • 5
    This question finally explained to me why deleters are useful. – Dan Dec 12 '14 at 16:29
  • 1
    The question uses unspecified/undefined behavior and is not guaranteed to work, because it takes the address of a function from the standard library. My solution is guaranteed to work, because it only calls free instead of taking its address. – PeterSom Apr 09 '18 at 14:21

5 Answers5

68

What you have is extremely likely to work in practice, but not strictly correct. You can make it even more likely to work as follows:

std::unique_ptr<char, decltype(std::free) *>
    t_copy { strdup(t), std::free };

The reason is that the function type of std::free is not guaranteed to be void(void*). It is guaranteed to be callable when passed a void*, and in that case to return void, but there are at least two function types that match that specification: one with C linkage, and one with C++ linkage. Most compilers pay no attention to that, but for correctness, you should avoid making assumptions about it.

However, even then, this is not strictly correct. As pointed out by @PeterSom in the comments, C++ allows implementations to e.g. make std::free an overloaded function, in which case both your and my use of std::free would be ambiguous. This is not a specific permission granted for std::free, it's a permission granted for pretty much any standard library function. To avoid this problem, a custom function or functor (as in his answer) is required.

Assuming it makes sense, it is possible to use a similar pattern with non-pointers?

Not with unique_ptr, which is really specific to pointers. But you could create your own class, similar to unique_ptr, but without making assumptions about the object being wrapped.

  • Using decltype() really underline what deleter you want to use and I like it and maybe it is worthy another question. However, is the type of ``extern "C" { void f(void*) }`` and ``void f(void*)`` different? I thought the difference was only in the symbol name. At very lease you can use the same kind of function pointers... – Paolo.Bolzoni Dec 12 '14 at 10:09
  • 4
    @Paolo.Bolzoni: Yes, they are different types, and you can't (portably) use the same pointer type. – Mike Seymour Dec 12 '14 at 10:37
  • 3
    @Paolo.Bolzoni A few implementations really complain about `extern "C" { void f(); } void (*fp)() = f;` with a message about assignment from an incompatible pointer type. I think I remember reading that HP's is one implementation that flat out rejects it, but I have no way to verify that. Sun's (now Oracle's) is another implementation that pays attention to it, but it allows such implicit conversions with a warning (and that I have seen myself). –  Dec 12 '14 at 10:47
  • 1
    You are right gentlemen. In my system ``std::is_same`` return true for the two ``decltype``s, but I checked the standard and yes different linkage means different type. There is always something to learn. – Paolo.Bolzoni Dec 12 '14 at 10:51
  • If the standard says that C functions have different types than C++ functions with the same signature, then it's the standard that's braindead in this regard. Whether a function has C or C++ linkage has nothing to do with how this function is implemented, the only difference is whether name mangling is applied or not. And the name of a thing has nothing to do with the type of the thing. Worse: there is only one syntax to write down the type of a function pointer. Taken literally, that means I can't even declare a C function pointer without resorting to `decltype`! – cmaster - reinstate monica Dec 12 '14 at 22:25
  • 1
    @cmaster They could legitimately have different calling conventions, the C convention for compatibility, the C++ convention a better one. As for avoiding `decltype`: create typedefs inside `extern "C"` blocks. But you'd be surprised how little it's needed. –  Dec 12 '14 at 22:46
  • @hvd No, I'm not surprised: Even though I know of some use cases where they are of crucial importance, I know how rarely these use cases come up. As to the calling conventions: Yes, you are right, a compiler *could* use the wording of the standard to use different calling conventions. Nevertheless, it *should* not do that as that would unnecessarily introduce a ton of extra complexity, and will likely break quite a few programs that rely on function pointers. I, for one, prefer the same syntax to mean the same thing in all contexts. That's what makes a language KISS. – cmaster - reinstate monica Dec 13 '14 at 00:05
  • @cmaster That's like arguing that compilers should not optimise signed integer arithmetic, should not assume array indices are less than the array length, etc. either. In general, if compilers can make programs faster by assuming no undefined behaviour, experience has shown that many will do so. At which point you're perfectly within your rights to choose not to use such an implementation: there are still others that don't do so. Different people have different wishes for their compilers. –  Dec 13 '14 at 00:46
  • @hvd Not really. It's more like arguing that the expression `42` should not be of type `int_c` in a C function, and of type `int_c++` in a C++ function, with no means to distinguish between the two other than the `decltype` keyword, and no means to convert the one into the other. This may sound abstruse, but in reality it's really very close to the issue with the function pointers. From a language design standpoint, such context sensitivity is an abomination that should be avoided at all costs. – cmaster - reinstate monica Dec 13 '14 at 09:39
  • @cmaster I thought you were arguing that the mere idea of language linkage was a horrible one, but your last comment seems more like you don't object to language linkage per se, but merely object to the lack of an explicit syntax for specifying language linkage of a function type. If that's what you mean, that's a fair point, and I think I would agree that the language would be improved with an explicit syntax. But you're wrong that `decltype` is needed. I put `decltype` in my answer because it's the easiest way here, not because it's the only way. –  Dec 13 '14 at 12:40
  • @hvd I object to both. One type `int` = good, two types `int_c` & `int_c++` = bad, two indistinguishable types `int` = evil. Same story for function pointers. Whether two types are necessary depends only on the implementation details (and it's not necessary for both `int`s and function pointers), how good/bad such a split is, is a question that's purely on the language level (and I believe it's a really bad idea). I have argued on both levels with different arguments, but the result is the same: Splitting function pointer types in this way is a bad design decision. – cmaster - reinstate monica Dec 14 '14 at 08:17
  • Using decltype(free) adds overhead of an extra pointer to the unique_ptr . See my answer for a solution without that space overhead. – PeterSom Apr 28 '17 at 10:23
  • As of C++2a the ISO standard makes it explicit (it was an implicit thing before) one is not allowed to take the address of almost all functions mentioned in the C++ standard (see https://wg21.link/p0551 , exceptions are for example the iostream manipulators). This includes all functions "inherited" from C. Thus the above example is invalid and my answer below is valid, because it calls free() and not takes its address. – PeterSom Apr 09 '18 at 14:17
  • @PeterSom You're overstating your point, but the point is valid. It's not UB (at least not yet, not even implicitly). It's not guaranteed to compile, but if it does compile, I don't see any way in which it's not guaranteed to behave as intended. Will edit. –  Apr 09 '18 at 14:53
  • sorry, it is "unspecified" in the standard. But if it compiles now and today it is not guaranteed to do so tomorrow, or with a different compiler. – PeterSom Apr 13 '18 at 11:44
43

The original question (and hvd's answer) introduce a per-pointer overhead, so such a unique_ptr is twice the size than one derived with std::make_unique. In addition, I would formulate the decltype directly:

std::unique_ptr<char, decltype(&std::free)>
    t_copy { strdup(t), &std::free };

If one has many of those C-API-derived pointers that extra space can be a burden hindering adoption of safe C++ RAII for C++ code wrapping existing POSIX style APIs requiring to be free()d. Another problem that can arise, is when you use char const in the above situation, you get a compile error, because you can not automatically convert a char const * to the Parameter type of free(void *).

I suggest to use a dedicated deleter type, not one built on the fly, so that the space overhead goes away and the required const_cast is also not a problem. A template alias then can easily be used to wrap C-API-derived pointers:

struct free_deleter{
    template <typename T>
    void operator()(T *p) const {
        std::free(const_cast<std::remove_const_t<T>*>(p));
    }
};
template <typename T>
using unique_C_ptr=std::unique_ptr<T,free_deleter>;
static_assert(sizeof(char *)==
              sizeof(unique_C_ptr<char>),""); // ensure no overhead

The example now becomes

unique_C_ptr<char const> t_copy { strdup(t) };

I will suggest that that mechanism should be made available in the C++ Core Guidelines support library ( maybe with a better naming ), so it can be made available for code bases transitioning to modern C++ ( success open ).

Community
  • 1
  • 1
PeterSom
  • 2,067
  • 18
  • 16
  • I know there are other more generic options to wrap deleter code into the unique_ptr's type, but they might be more involved and not as simple to understand and implement. – PeterSom Apr 26 '17 at 06:10
  • I like this answer and agree with most of it, except for saying my answer introduces overhead. It does not: it has the same overhead as what's in the question. I merely did not address that aspect (and hadn't thought of it, at least not when writing my answer). –  Apr 28 '17 at 16:13
  • 1
    Yes, the original question already had the unnecessary overhead. And I was adding my answer, becausethat overhead was used as an excuse for not using unique_ptr for the purpose, and because my suggestion was objected on the std mailing list as something to put into the C++ Standard. And yes, until a week ago,I also hadn't thought of this solution consciously. – PeterSom Apr 28 '17 at 21:39
  • If you agree that my answer is not what introduces the overhead, please edit your answer to no longer make that claim. For instance: "The answer <...> does not fix the per-pointer overhead, so..." would seem to me to not detract from the point of your answer. –  Apr 29 '17 at 08:29
  • see above. The original question and answer are undefined behavior, because they take the address of a standard library function, which is not defined in the standard. Implementors are free to provide other overloads or have wider parameter lists providing default arguments. Just calling a function from the standard library is guaranteed to work. – PeterSom Apr 09 '18 at 14:20
  • Hm. Wouldn't it be good for generality to invoke the dtor just in case it's not trivial? Otherwise, you could just use a void-pointer instead of a template... – Deduplicator Apr 09 '18 at 17:30
  • s/undefined/unspecified/ behavior in my comment above. – PeterSom Apr 13 '18 at 11:39
  • I do not understand Deduplicator's comment. – PeterSom Apr 13 '18 at 11:39
  • Won't `template void operator()(volatile const T *p) const { std::free(const_cast(p)); }` be faster to compile and shorter? – Doncho Gunchev Oct 06 '20 at 12:32
8

Assuming it makes sense, it is possible to use a similar pattern with non-pointers? For example for the POSIX's function open that returns an int?

Yes, it can be done. You need a "pointer" type that satisfies the NullablePointer requirements:

struct handle_wrapper {

    handle_wrapper() noexcept : handle(-1) {}
    explicit handle_wrapper(int h) noexcept : handle(h) {}
    handle_wrapper(std::nullptr_t)  noexcept : handle_wrapper() {}

    int operator *() const noexcept { return handle; }
    explicit operator bool() const noexcept { return *this != nullptr; }

    friend bool operator!=(const handle_wrapper& a, const handle_wrapper& b) noexcept {
        return a.handle != b.handle;
    }

    friend bool operator==(const handle_wrapper& a, const handle_wrapper& b) noexcept {
        return a.handle == b.handle;
    }

    int handle;
};

Note that we use -1 as the null handle value here because that's what open() returns on failure. It's also very easy to templatize this code so that it accepts other handle types and/or invalid values.

Then

struct posix_close
{
    using pointer = handle_wrapper;
    void operator()(pointer fd) const
    {
        close(*fd);
    }
};

int
main()
{
    std::unique_ptr<int, posix_close> p(handle_wrapper(open("testing", O_CREAT)));
    int fd = *p.get();
}

Demo.

T.C.
  • 133,968
  • 17
  • 288
  • 421
  • Sounds correct, but if I need to write a class like that probably using _finally_ and a lambda is easier to read... ( _finally_ C++ implementation: http://stackoverflow.com/a/25510879/1876111 ) – Paolo.Bolzoni Dec 14 '14 at 13:05
  • Better yet, specialise `std::default_delete` and create a function `wrap_open`, then a user can write simply `auto p = std::make_unique(wrap_open("testing", O_CREAT));`. – Konrad Rudolph Jan 15 '19 at 14:16
6

Assuming it makes sense, it is possible to use a similar pattern with non-pointers? For example for the POSIX's function open that returns an int?

Sure, using Howard's Hinnant tutorial on unique_ptr, we can see a motivating example:

// For open
#include <sys/stat.h>
#include <fcntl.h>

// For close
#include <unistd.h>

// For unique_ptr
#include <memory>

int main()
{
    auto handle_deleter = [] (int* handle) {
        close(*handle);
    };

    int handle = open("main.cpp", O_RDONLY);
    std::unique_ptr<int, decltype(handle_deleter)> uptr
        { &handle, handle_deleter };
}

Alternatively you can use a functor instead of a lambda:

struct close_handler
{
    void operator()(int* handle) const
    {
        close(*handle);
    }
};

int main()
{
    int handle = open("main.cpp", O_RDONLY);
    std::unique_ptr<int, close_handler> uptr
        { &handle };
}

The example can be further reduced if we use a typedef and a "factory" function.

using handle = int;
using handle_ptr = std::unique_ptr<handle, close_handler>;

template <typename... T>
handle_ptr get_file_handle(T&&... args)
{
    return handle_ptr(new handle{open(std::forward<T>(args)...)});
}

int main()
{
    handle_ptr hp = get_file_handle("main.cpp", O_RDONLY);
}
  • 7
    Since you're now storing a pointer to the handle, rather than the handle itself, there are possible problems with the object's lifetime if `handle` is destroyed before the `unique_ptr` is. You attempt to address that by using `new handle`, but in that case, don't forget that `close_handler` will need to not just close the handle, it also needs to free the memory, or you've got a leak. –  Dec 12 '14 at 11:22
  • 2
    In other words the `operator()` needs a `delete handle` to work correctly. – ratchet freak Dec 12 '14 at 13:47
  • This works perfect with win32 functions too, e.g.: `std::unique_ptr argv(CommandLineToArgvW(commandLine, &argc), ::LocalFree)` – kiewic Mar 08 '17 at 21:28
5

Just to update the answer: C++20 allows lambdas in unevaluated contexts (like decltype), so there is no longer a need to even write a structure to call std::free to take advantage of empty base optimization. The following will do.

std::unique_ptr<T, decltype([](void *p) { std::free(p); })> ptr;

The trick is that such a lambda is default constructible.

Hunter Kohler
  • 1,885
  • 1
  • 18
  • 23