2

Previously:

Proper way close WinAPI HANDLEs (avoiding of repeated closing)

My question is: what if CreateFile returned a bool instead of a HANDLE, and for the output there was a pointer? Invented example:

HANDLE handle;

if (CreateFile(&handle, "filename", ...) == true) {
 //...
}

Is there a nice syntax to use this in C++ with the provided RAII class? So that I could put my object as the first argument instead of the pointer.

Community
  • 1
  • 1
J. Doe
  • 73
  • 9
  • No, only ugly syntax. You fumble an explicit Attach() call. – Hans Passant Mar 07 '17 at 14:30
  • 1
    COM smart pointers (like ATL::CComPtr) are made to work with this kind of pattern, where the desire result is an "output parameter" because the function actually returns a status code. – Adrian McCarthy Mar 07 '17 at 19:12
  • And they do that by overriding `operator&`, for instance [`CComPtrBase::operator &`](https://msdn.microsoft.com/en-us/library/31k6d0k7.aspx). – Remy Lebeau Mar 07 '17 at 20:07
  • And `CComPtrBase::operator&` has an assertion because it's dangerous and frequently led to leaks, which defeats the purpose of an RRID wrapper. – Adrian McCarthy Mar 07 '17 at 21:02

3 Answers3

1

In this situation, I would simply add an overrriden operator& to the RAII class, eg:

template< class traits >
class HandleWrapper {
...
public:
    ...
    traits::HandleType* operator&() { return &FHandle; }
};

Then use it like this:

HandleWrapper<KernelHandleTraits> handle;

if (CreateFile(&handle, "filename", ...) == true) {
    ...
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Isn't it trickier than that? Don't you have to deal with the possibility that the HandleWrapper already holds a valid handle? When the caller uses `operator&` and then assigns through the pointer, HandleWrapper needs to release the original resource (if any). And, ideally, if the caller doesn't assign through the pointer, the wrapper should still hold the original resource. I suspect the real trick is it to have `operator&` return a proxy object that's convertible to `HandleType *` and overloads assignment. – Adrian McCarthy Mar 07 '17 at 19:22
  • You can certainly implement `operator&` to release an existing resource, or even throw an exception if a resource is already held. That is your choice. If you do not pass wrappers around, you just use it for local cleanup, you don't have to worry about this. This was just an example to answer the question of how to pass a wrapper to a function that expects a pointer to a resource. You can make it more complicated if you need to. – Remy Lebeau Mar 07 '17 at 20:00
  • Personally, I wouldn't use `operator&` for this. A named function seems a much better idea to me. – Martin Bonner supports Monica Mar 08 '17 at 06:45
0

What you want, is a function which calls CreateFile, throws if it returns false, and returns the handle. Something like:

HANDLE createFileOrThrow(const std::string& filename, ... )
{
    HANDLE handle;
    if (!CreateFile(&handle, filename.c_str(), ...) {
        throw suitable_exception_type();
    }
    return handle;
}

HandleWrapper<KernelHandleTraits> hFile(CreateFileOrThrow("filename"));

In this particular example, you could make the function be a lambda which you immediately call, but you might want to do this often.

Alternatively:

HandleWrapper<KernelHandleTraits> createFileOrThrow(const std::string& filename, ... )
{
    HANDLE handle;
    if (!CreateFile(&handle, filename.c_str(), ...) {
        throw suitable_exception_type();
    }
    return HandleWrapper<KernelHandleTraits>(handle);
}

HandleWrapper<KernelHandleTraits> hFile = CreateFileOrThrow("filename");

That last option does require HandleWrapper to have a move constructor though.

  • Your second example requires a copy constructor, and handles are not always copyable, or require specialized functions to perform copies, so you would have to account for that in the `traits` class. – Remy Lebeau Mar 07 '17 at 20:05
  • You don't need a *copy* constructor - but you do need a *move* constructor. A move constructor for HandleWrapper is straightforward, and allows you to have vectors of wrapped handles etc. But good point. – Martin Bonner supports Monica Mar 08 '17 at 06:43
0

You can certainly achieve the goal with an overload of operator& as others have mentioned, but that's fraught with risk that defeats the purpose of using the wrapper in the first place.

Consider how you might use a hypothetical HandleWrapper with a simple overload of operator& that just returns a pointer to the internal handle:

HandleWrapper<KernelHandleTraits> handle;

if (CreateFile(&handle, "first", ...) == true) {
  // process the first file...
}
if (CreateFile(&handle, "second", ...) == true) {
  // process the second file...
}

This leaks the handle to the first file, because the operator& overload lets you do a backdoor assignment. Note that the wrapper class doesn't have an assignment operator specifically so that it doesn't have to cope with the complications of re-using a wrapper to track a second resource once you're done with the first.

You could extend the wrapper to do something smarter if it already contains a resource but the class gets more complex and you get farther from the idea of RAII.

If we're splitting hairs, the HandleWrapper code referenced applies RRID (resource release is destruction) rather than RAII (resource allocation is initialization).

  • An RRID class is a wrapper you can transfer ownership of an already allocated resource to (in order to ensure that it's released later).
  • An RAII class is one whose lifetime is exactly the same as the resource it wraps. It owns the resource the moment it comes into existence and dies when the resource is released.

If you have a function that returns resources as "output parameters," you can write an adapter function:

HANDLE CreateFileAdapter(const char * filename, ...) {
  HANDLE handle;
  return CreateFile(&handle, "filename", ...) ? handle : INVALID_HANDLE_VALUE;
}

You can then use your HandleWrapper as-is by calling the adapter function:

HandleWrapper<KernelHandleTraits> handle(CreateFileAdapter("filename", ...));

A strict RAII solution would encapsulate the allocation of the resource in a constructor just as it encapsulates the deallocation in its destructor. You can build an RAII wrapper on the back of an RRID wrapper and an adapter function:

class FileHandleWrapper : public HandleWrapper<KernelHandleTraits> {
  public:
    FileHandleWrapper(const char * file_name, ...) :
      HandleWrapper(CreateFileAdapter(file_name, ...) {}

  private:
    static HANDLE CreateFileAdapter(const char *file_name, ...) {
      HANDLE handle;
      return CreateFile(&handle, "filename", ...) ?
          handle : INVALID_HANDLE_VALUE;
    }
};

Note that I've made the adapter function a private static, to ensure that nobody uses it in an unsafe manner.

Adrian McCarthy
  • 45,555
  • 16
  • 123
  • 175