Hello. Thanks for feedback, Mathias! Mathias Gaunard wrote:

You might want to use proper move emulation in the absence of C++0x, as is provided by Boost.Move. There is no such library Boost.Move. Do you mean to implement move like Boost.Thread does? If so. I think it is not necessary because one can just write "underlying_resource_type" instead of "thread_move_t" and ".release()" instead ".move()".

Also it seems your implementation doesn't support throwing move constructors. If copy constructor or move constructor can throw an exception the only function that works incorrect is release. There is no way to implement it in exception safe manner in this case.

Note that usually this is not a problem. Usually underlying_resource_type is not something complex, but rather it is just int or void* or something like that. So its copy and move constructors do not throw exceptions.

Ivan Sorokin wrote:

Hello.

Thanks for feedback, Mathias!

Mathias Gaunard wrote:

...

You might want to use proper move emulation in the absence of C++0x, as is provided by Boost.Move. There is no such library Boost.Move.

It is in the review queue to be included as a first-class Boost citizen, but it seems it is already used by Boost.Interprocess and shipped with it.

If copy constructor or move constructor can throw an exception the only function that works incorrect is release.

My bad, I assumed the operator= was implemented as you described in the documentation (clear current resource first), but you're actually using a temporary then swap, which means the new resource is moved *before* the old resource is released.

Ivan Sorokin wrote:

Mathias Gaunard wrote:

...

...

...

You might want to use proper move emulation in the absence of C++0x, as is provided by Boost.Move. There is no such library Boost.Move. It is in the review queue to be included as a first-class Boost citizen, but it seems it is already used by Boost.Interprocess and shipped with it.

I've implemented my library over Boost.Move: http://rain.ifmo.ru/~sorokin/rcpp/boost_move.diff

I have three questions.

1 Is it possible to specialize is_movable<T> for primitive types (e.g. int, void*, etc) to return true? 2 Did I properly specialize has_nothrow_move?

I think is_movable is supposed to be used to detect whether the type uses Boost.Move. Your specialization of has_nothrow_move appears correct at first glance.

3 When will Boost.Move become a part of boost?

When it gets reviewed. It could be said, however, that the review system in place is extremely slow and that it's going to take years (literally).

AMDG Daniel Trebbien wrote:

What is the safe_bool stuff for; or, why have you implemented `operator sb_type() const` rather than `operator bool() const` for evaluating a `resource` as a `bool`?

This is a common idiom to prevent problems from the fact that bool can convert to int. In Christ, Steven Watanabe

What kind of HANDLE that can be returned from DuplicateHandle should not be disposed by CloseHandle?

The only time I should call `CloseHandle` on the `HANDLE` result from `DuplicateHandle` is if the `DuplicateHandle` call succeeded. This is because the MSDN documentation does not assert that the `HANDLE` resulting from `DuplicateHandle` will never be `NULL` or `INVALID_HANDLE_VALUE`.

...

Essentially the underlying resource type in this case is a pair of a `HANDLE` and `BOOL`, where the "invalid value" is a pair with the `BOOL` set to `FALSE` and an indeterminate value of the `HANDLE`. In order to use the `rcpp::resource` template, I would need to override the `==` operator so that equality of the resource means equality of the `BOOL` member only, which is not ideal. I think that an `is_invalid` member would do the trick rather nicely, however. Why do not use boost::optional<HANDLE> as underlying resource type?

I hadn't thought of this. Seems like it would work...

...

Also, I think that it would be nice to have a `shared_resource` template. I have an application in mind where I would like to create a shared `HANDLE` that is only closed if it is invalid and all references to it have been destroyed (a shared reference count reaches 0). I thought about that. I don't think it has wide application. Moreover it can be emulated with boost::shared_ptr<rcpp::resource>. Is it worth while?

I think so because `boost::shared_ptr` adds a level of indirection that is unnecessary.

What do you think about the following functions?

rcpp::win::file_handle clone(rcpp::win::file_handle const & f) { HANDLE h; BOOL r = ::DuplicateHandle(GetCurrentProcess(), f.get(), GetCurrentProcess(), &h, 0, FALSE, DUPLICATE_SAME_ACCESS);

if (r == FALSE) return rcpp::win::file_handle(); // or throw an exception

return rcpp::win::file_handle(h); }

rcpp::win::kernel_handle clone(rcpp::win::kernel_handle const & f) { HANDLE h; BOOL r = ::DuplicateHandle(GetCurrentProcess(), f.get(), GetCurrentProcess(), &h, 0, FALSE, DUPLICATE_SAME_ACCESS);

if (r == FALSE) return rcpp::win::kernel_handle(); // or throw an exception

return rcpp::win::kernel_handle(h); }

In `clone(rcpp::win::file_handle const &)`, what if `h` is `INVALID_HANDLE_VALUE` and `r` is not `FALSE` (`DuplicateHandle` succeeded, but the resulting `HANDLE` was `INVALID_HANDLE_VALUE`)? Then the function would return an "empty" `rcpp::win::file_handle` and the `INVALID_HANDLE_VALUE` `HANDLE` would never be closed. Similarly, if `DuplicateHandle` could set `h` to `NULL` in `clone(rcpp::win::kernel_handle const &)`, then the `NULL` `HANDLE` would never be closed. I know, you're thinking that this is picky. Yes, it's really picky, but I can't help from thinking about this case, especially because the MSDN documentation does not definitively state that this can not happen. I also looked at the documentation for `CloseHandle` to see if it can not accept `NULL` or `INVALID_HANDLE_VALUE`, but there is nothing on that issue, either. (If `CloseHandle` did not accept `NULL`, for example, then `DuplicateHandle` could never use `NULL` for the resulting `HANDLE` value.) I like the solution that you had, which is to use `boost::optional<HANDLE>` with invalid value `boost::none`.

On Feb 28, 2010, at 6:13 PM, Daniel Trebbien wrote:

[...] I know, you're thinking that this is picky. Yes, it's really picky, but I can't help from thinking about this case, especially because the MSDN documentation does not definitively state that this can not happen. I also looked at the documentation for `CloseHandle` to see if it can not accept `NULL` or `INVALID_HANDLE_VALUE`, but there is nothing on that issue, either. (If `CloseHandle` did not accept `NULL`, for example, then `DuplicateHandle` could never use `NULL` for the resulting `HANDLE` value.)

Calling 'CloseHandle' with a value of NULL or INVALID_HANDLE_VALUE usually simply results in an access violation. You may be able to catch it with SEH but I have never tried... Ciao, Andreas