Comments 17
Рано или поздно мы заметим, что большинство классов-обёрток незначительно отличаются друг от друга: как правило единственное отличие – это функция освобождения ресурса. Подобный подход провоцирует подверженное ошибкам повторное использование кода в стиле «копировать/вставить».
Логично параметризовать эту функцию, зачем копипастить?
С другой стороны, мы видим здесь отличную возможность для обобщения, которая подводит нас к следующему варианту – использованию умных указателей.
Что в имени тебе моем?(с) Пушкин А.С.
Берем сущность которая не является указателем, не имеет ничего с ним общего, и обобщаем до указателя. За что и поплатились:
Причина ошибки в приведённом примере – нарушение концепции NullablePointer типом Handle. Вкратце, модель концепции NullablePointer, должна являться объектом, поддерживающим семантику указателей, и в частности допускающим сравнение с nullptr. Наш Handle, определённый как синоним для int, не является таким объектом. Как следствие, мы не можем использовать std::unique_ptr, для вещей наподобие файловых дескрипторов POSIX или ресурсов OpenGL.
«создаем себе трудности и успешно их преодолеем»
Получили архитектурную ошибку.
+3
Берем сущность которая не является указателем,
Да ладно? А что же это тогда? Число в системной таблице, указывающее на ресурс, не?
Логично параметризовать эту функцию, зачем копипастить?
А вот с этим соглашусь — тоже сразу приходит в голову такая мысль.
0
Может таблица, может идентификатор узла в списка или дерева.
А может это не идентификатор указывающий на ресурс, а некий объект. Принцип инкапсуляции и слоистая архитектура скрывают от меня эту информацию.
Из подобных примеров у нас есть std::lock_guard, который так же работает в RAII-стиле(забавно, там так же в пример приводят lock_guard).
А может это не идентификатор указывающий на ресурс, а некий объект. Принцип инкапсуляции и слоистая архитектура скрывают от меня эту информацию.
Из подобных примеров у нас есть std::lock_guard, который так же работает в RAII-стиле(забавно, там так же в пример приводят lock_guard).
0
Был бы сам объект, так бы его и назвали объектом, а не «ручкой» (тут как раз подходит цитата Пушкина). А раз назвали, то для вас это — указатель. Даже если он «указывает» сам в себя (т.е. являться не указателем, а реальным объектом). Вот скажите, url-адрес это объект? А является ли он указателем? Чтобы прочитать статью на хабре, вам url нужен именно как указатель, и как указатель вы его и используете, не заботясь о том, что это объект, у которого можно посчитать длину, выяснить, по https вы пришли или http, и прочую, безусловно важную, но в данном случае, ненужную информацию.
Принцип инкапсуляции в данном случае скрывают от вас, как устроен указатель, но то что это указатель — вам говорят совершенно четко.
Принцип инкапсуляции в данном случае скрывают от вас, как устроен указатель, но то что это указатель — вам говорят совершенно четко.
-3
В классе присутствуют только конструктор перемещения и оператор перемещения, однако инициализация и присваивание ресурса происходит по значению, что может грозить серьезными ошибками в некоторых случаях, что неприемлемо для «универсальной» обертки.
Пример с std::unique_ptr принципиально некорректен, а спекуляция с использованием std::remove_pointer указывает на полное непонимание принципа его работы. В случае std::unique_ptr работу с ресурсом необходимо оборачивать в работу над указателем на ресурс, а не маскировать ресурс под указатель.
Кстати последняя ревизия Generic Scope Guard and RAII Wrapper — N4189.
Очень жаль, что такой некачественный материал публикуется в журналах, можно было бы и проверять материал до публикации. Этим грешат и некоторые книги, а потом эти ошибки из кода примеров оказываются в реальных проектах.
Пример с std::unique_ptr принципиально некорректен, а спекуляция с использованием std::remove_pointer указывает на полное непонимание принципа его работы. В случае std::unique_ptr работу с ресурсом необходимо оборачивать в работу над указателем на ресурс, а не маскировать ресурс под указатель.
Кстати последняя ревизия Generic Scope Guard and RAII Wrapper — N4189.
Оригинальная статья опубликована в выпуске 126 журнала Overload (апрель 2015).
Очень жаль, что такой некачественный материал публикуется в журналах, можно было бы и проверять материал до публикации. Этим грешат и некоторые книги, а потом эти ошибки из кода примеров оказываются в реальных проектах.
0
спекуляция с использованием std::remove_pointer указывает на полное непонимание принципа его работы. В случае std::unique_ptr работу с ресурсом необходимо оборачивать в работу над указателем на ресурс, а не маскировать ресурс под указатель.
А можно здесь поподробней? Я использую подобный трюк и хотелось бы узнать в чём-же моё непонимание std::unique_ptr. На примере HANDLE, как бы поступили Вы?
0
Одно дело что вы используете «трюк» (по сути «хак»), а совсем другое, что он преподносится как единственно верный вариант.
#include <memory>
using Handle = int;
Handle CreateHandle() { Handle h{ -1 }; /*...*/ return h; }
void CloseHandle(Handle& h) { /* ... */ }
struct HandleDeleter {
void operator()(Handle* h) { CloseHandle(*h); delete h; }
};
using ScopedHandle = std::unique_ptr<Handle, HandleDeleter>;
int main() {
ScopedHandle h{ new Handle(CreateHandle()) };
}
0
Ну, снова таки — из приведённого Вами документа, видно, что автор разбивает ресурсы на «pointer handle type» и «handle types that are not pointers». Для работы с ресурсами первого типа, как раз таки, приводится прекрасный пример использования
Для ресурсов второго типа («handle types that are not pointers») — используется уже новый, предлагаемый API — т.е., я хочу сказать, что простых решений с использованием существующей библиотеки C++ (как в случае с «pointer handle type») — нет.
Так вот Вы же приводите пример, который пытается решить проблему «handle types that are not pointers» c помощью того же
std::unique_ptr
для управления ресурсом (конечно, есть и недостатки).Пример из N4189
typedef void *HANDLE; // System defined opaque handle type
typedef unsigned long DWORD;
#define INVALID_HANDLE_VALUE reinterpret_cast<HANDLE>(-1)
// Can’t help this, that’s from the OS
// System defined APIs
void CloseHandle(HANDLE hObject);
HANDLE CreateFile(const char *pszFileName,
DWORD dwDesiredAccess,
DWORD dwShareMode,
DWORD dwCreationDisposition,
DWORD dwFlagsAndAttributes,
HANDLE hTemplateFile);
bool ReadFile(HANDLE hFile,
void *pBuffer,
DWORD nNumberOfBytesToRead,
DWORD*pNumberOfBytesRead);
// Using std::unique ptr to ensure file handle is closed on scope-exit:
void CoercedExample()
{
// Initialize hFile ensure it will be ”closed” (regardless of value) on scope-exit
std::unique_ptr<void, decltype(&CloseHandle)> hFile(
CreateFile("test.tmp",
FILE_ALL_ACCESS,
FILE_SHARE_READ,
OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL,
nullptr),
CloseHandle);
// Read some data using the handle
std::array<char, 1024> arr = { };
DWORD dwRead = 0;
ReadFile(hFile.get(), // Must use std::unique ptr::get()
&arr[0],
static_cast<DWORD>(arr.size()),
&dwRead);
}
Для ресурсов второго типа («handle types that are not pointers») — используется уже новый, предлагаемый API — т.е., я хочу сказать, что простых решений с использованием существующей библиотеки C++ (как в случае с «pointer handle type») — нет.
Так вот Вы же приводите пример, который пытается решить проблему «handle types that are not pointers» c помощью того же
std::unique_ptr
, и говорите, что использовать std::unique_ptr
для «pointer handle type» — спекуляция.0
В вашем примере происходит лишнее выделение памяти под хендл. При освобождении ресурса функция CloseHandle может бросить исключение и память, выделенная под хендл, не будет освобождена.
0
В вашем примере происходит лишнее выделение памяти под хендл.
Именно поэтому unique_ptr и не является хорошим инструментом для реализации scope_guard.
При освобождении ресурса функция CloseHandle может бросить исключение и память, выделенная под хендл, не будет освобождена.
Тут у нас есть несколько решений — потребовать условия noexcept(CloseHandle(*h)), просто проигнорировать исключение посредством try-catch, или передать это исключение для дальнейшей обработки:
struct HandleDeleter {
void operator()(Handle* h) {
std::exception_ptr ex;
try {
CloseHandle(*h);
} catch (...) {
ex = std::current_exception();
}
delete h;
if (ex){
std::rethrow_exception(ex);
}
}
};
Что, впрочем, не является лучшей идеей, учитывая, что deleter будет вызван в деструкторе unique_ptr.
0
По условиям задачи, CloseHandle бросать исключения не может так как реализована на Си.
0
Где же это написано, что функция CloseHandle написана на C? К тому же в Windows у нас есть SEH
0
По условиям задачи мы пишем удобную обертку для низкоуровневого C-like API, откуда там C++ исключения?
Взаимодействие SEH и исключений в C++ это тема для отдельного разговора. Если я правильно помню, в Winapi используются коды возврата а не SEH чуть менее чем во всех функциях?
Но в целом вы конечно правы)
Взаимодействие SEH и исключений в C++ это тема для отдельного разговора. Если я правильно помню, в Winapi используются коды возврата а не SEH чуть менее чем во всех функциях?
Но в целом вы конечно правы)
0
Во-первых, такие вещи должны быть обязательно обложены тестами, а, во-вторых, в дизайне и коде есть пара моментов. Самое главное в дизайне —
Дальше коментарии (возможно, тянут на доп статью) пойдут про
— я бы сделал
— заранее неизвестно, что инициализация ресурса в значение по-умолчанию есть правильно, из этого вытекают всякие моменты почти во всех остальных методах
— так же из первого довольно сильно могут усложниться реализации всяких специфичных производных (речь о
— в качестве конструктора по-умолчанию кажется более логично использовать конструктор с одним аргументом, который принимает resource, и значением по-умолчанию
— нет возможности узнать открыт (валидный) handle или нет
— по моему опыту, публичный метод
— иногда надо иметь возможность получить указатель на уже открытый handle (например, чтобы сложить его в какой-нибудь массив), побочный эффект (side effect), очищающий ресурс тут немного некстати. Это действительно очень спорное решение. В зависимости от ситуации тут может лучше вернуть
— неплохо было бы иметь
—
— для
— отсутствие
В этой связи я бы ввёл какой-нибудь traits, например,
Для документации и тем более для статьи обязательно нужно оговорить все моменты закрытия ресурса. Во-первых, состояние после закрытия должно быть рабочим (в Вашем примере неплохо было бы присвоить хендлу спецальное значение), во-вторых, закрытие должно быть строго детерминировано, а, в-третьих, не должны вызываться закрывающие методы от предыдущих ресурсов (у Вас с этим все хорошо на этам дизайна, теги помогают, а мне повезло поработать и с таким кодом).
ScopedHandle
неудачный пример для производной от Вашего Resource
.Дальше коментарии (возможно, тянут на доп статью) пойдут про
ScopedHandle
, поскольку статья выше в основном об этом.— я бы сделал
ScopedHandle
без предложенного Resource
, без std::unique_ptr
и ограничился бы поддержкой только тривиальных хендлов (static_assert(std::is_trivial<Handle>::value, "The Handle type should be trivial.");
). Это существенно всё упрощает.— заранее неизвестно, что инициализация ресурса в значение по-умолчанию есть правильно, из этого вытекают всякие моменты почти во всех остальных методах
— так же из первого довольно сильно могут усложниться реализации всяких специфичных производных (речь о
using ScopedHandle = Resource<struct HandleTag, Handle>
, кстати, неплохо было бы добавить final, вряд ли тут будет к месту наследование, и так довольно сложный фундаментальный класс)— в качестве конструктора по-умолчанию кажется более логично использовать конструктор с одним аргументом, который принимает resource, и значением по-умолчанию
— нет возможности узнать открыт (валидный) handle или нет
— по моему опыту, публичный метод
close
оказался очень полезен— иногда надо иметь возможность получить указатель на уже открытый handle (например, чтобы сложить его в какой-нибудь массив), побочный эффект (side effect), очищающий ресурс тут немного некстати. Это действительно очень спорное решение. В зависимости от ситуации тут может лучше вернуть
nullptr
, если ресурс уже открыт, вернуть указатель на ресурс без изменений, очистить ресурс (как у Вас) или бросить исключение (пока что во всех моих проектах, я был против варианта бросания исключения).— неплохо было бы иметь
Resource& operator=(ResourceType h)
—
operator const ResourceType&() const noexcept { return resource_; }
тут вопрос с константностью, допустим у нас константный объект, тогда мы можем получить ресурс (хендл), и изменить его, так как он скопируется при передаче в функцию. Тут философский вопрос о побитовой и логической константности, но все же упомянуть стоит. Я бы лично сделал неконстантным, больше помощи от компилятора.— для
Resource& operator=(Resource&& other)
я бы воспользовался swap idiom, но так тоже хорошо.— отсутствие
operator bool() const
и размышлений на эту тему.В этой связи я бы ввёл какой-нибудь traits, например,
template<typename Handle, Handle InvalidValueT>
struct BaseScopedHandleTraits {
typedef Handle Handle;
typedef Handle* HandlePtr;
// required to be implemented
//static bool close(Handle h)
static Handle initialValue() {
return InvalidValueT;
}
static bool test(Handle value) {
return InvalidValueT != value;
}
};
template<HANDLE InvalidValue>
struct WindowsHandleTraits : BaseScopedHandleTraits<HANDLE, InvalidValue> {
static bool close(Handle h) {
return 0 != CloseHandle(h);
}
};
Для документации и тем более для статьи обязательно нужно оговорить все моменты закрытия ресурса. Во-первых, состояние после закрытия должно быть рабочим (в Вашем примере неплохо было бы присвоить хендлу спецальное значение), во-вторых, закрытие должно быть строго детерминировано, а, в-третьих, не должны вызываться закрывающие методы от предыдущих ресурсов (у Вас с этим все хорошо на этам дизайна, теги помогают, а мне повезло поработать и с таким кодом).
+1
Плохая идея — изображать из хэндлов указатели, только ради повторного использования unique/shared_ptr. Хотя бы потому, что сигнальные значение указателя — это 0, а сигнальные значение хэндла может быть каким угодно: 0, -1, да хоть 0xDEADFACE. Опять же, «указуемый тип» хэндла не всегда существует. Ну и риск случайно сделать shared_ptr<remove_pointer> с дефолтным делетером вместо специального.
Поэтому для всех будет лучше не впихивать невпихиваемое, а делать отдельные классы умных обёрток.
Поэтому для всех будет лучше не впихивать невпихиваемое, а делать отдельные классы умных обёрток.
+1
Sign up to leave a comment.
Управление ресурсами с помощью явных специализаций шаблонов