Comments 55
Уже лучше, но мы не можем контролировать, хотим ли мы захватывать внешние переменные по значению или по ссылке, что важно в некоторых случаях.
Таких случаев нет
Также ваш тип копирует лямбду которую захватывает(а не мувает), что дополнительно создает проблемы
Таких случаев нет
Ого, как категорично!
что дополнительно создает проблемы
Не помешало бы привести конкретный пример, демонстрирующий озвученные проблемы.
Ого, как категорично!
приведите пример, когда без явного захвата в scope exit у вас не получается что-то
Не помешало бы привести конкретный пример, демонстрирующий озвученные проблемы.
я не знаю что тут можно демонстрировать, если делается копирование вместо мува, вам доказать, что некоторые типы не умеют копироваться или то что копировать это дороже чем мувать?
Не помешало бы привести конкретный пример, демонстрирующий озвученные проблемы.
Попробуйте передать в качестве параметра absl::AnyInvocable. Внезапно, он не копируется.
Современные фичи С++ это хорошо, правильно. Но если уж исходить из юзабилити, вполне понятный вариант второй. Проверил, прочитал, закрыл. Никто же не мешает отделить LoadLibraryA и FreeLibrary в класс и дергать его в данном методе при выходе из функции сработает деструктор. Написать шаблон с HANDLE, который в деструкторе будет всегда CloseHandle(file) это делать.
Помню, вроде на rsdn подобный код был, типа заворачиваем WinApi в ООП с конструкторами и деструкторами.
bool MakeDumpToFile(DWORD pid, PCWCHAR filename)
{
HMODULE dbgdll = LoadLibraryA("dbghelp.dll");
if (dbgdll)
{
auto pfnMiniDumpWriteDump = (decltype(&MiniDumpWriteDump)) GetProcAddress(dbgdll, "MiniDumpWriteDump");
if (pfnMiniDumpWriteDump)
{
HANDLE proc = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid);
if (proc)
{
HANDLE file = CreateFileW(filename, GENERIC_WRITE, NULL, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
if (file && file != INVALID_HANDLE_VALUE)
{
bool result = pfnMiniDumpWriteDump(proc, pid, file, MiniDumpNormal, NULL, NULL, NULL);
CloseHandle(file);
return result;
}
CloseHandle(proc);
}
}
FreeLibrary(dbgdll);
}
return false;
}
defer { free(p); };
Это как в Swift (кроме точек с запятой)
Придумал, ещё вариант по старинке:) Класс Library и Handler дергают деструктор FreeLibrary и CloseHandle.
bool MakeDumpToFile(DWORD pid, PCWCHAR filename)
{
Library dbgdll("dbghelp.dll");
if (dbgdll.Ok())
{
auto pfnMiniDumpWriteDump = (decltype(&MiniDumpWriteDump)) dbgdll.GetAddress("MiniDumpWriteDump");
if (pfnMiniDumpWriteDump)
{
Handler<HANDLE> proc(procOpenProcess(PROCESS_ALL_ACCESS, FALSE, pid));
if (proc.Ok())
{
Handler<HANDLE> file(CreateFileW(filename, GENERIC_WRITE, NULL, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL));
if (file.Ok() && file.Get() != INVALID_HANDLE_VALUE)
{
bool result = pfnMiniDumpWriteDump(proc.Get(), pid, file.Get(), MiniDumpNormal, NULL, NULL, NULL);
return result;
}
}
}
}
return false;
}Я же упомянул об этом прямо во втором предложении в статье:
Создание достойных RAII-врапперов для каждого используемого сишного API не всегда практично
Для полноты картины вам нужно учитывать объём кода всех созданных вами врапперов, а так же то, что они будут не везде применимы. Например, после CreateProcess у вас в структуре PROCESS_INFORMATION pi будут сырые хендлы pi.hProcess и pi.hThread, которые нужно закрыть, и вот вам уже нужно писать свой враппер для CreateProcess. Так вы быстро скатитесь к разработке целого C++ фреймворка на все случаи жизни вместо С-подобного низкоуровневого кода, о котором идёт речь в статье.
Так вам не нужна обёртка для каждого вызова, вам нужна обёртка для каждого способа освобождения ресурса (CloseHandle, RegCloseKey, LsaClose, NetApiBufferFree, CloseThreadpool, что там ещё из массового). Причём подавляющее большинство ресурсов освобождаются стандартно, без приколов а ля CreateProcess. Мне кажется, вы сильно переоцениваете количество работы, которое нужно сделать базово, закатывать всё винапи целиком в raii нужно примерно так никому. Никому, кроме майков, т.е., так что берите и пользуйтесь: https://github.com/microsoft/wil.
Выглядит потрясающе. Честно говоря, еще со школьных времен, когда только-только начинал изучать программирование, никогда не понимал, почему в любой более-менее сложной программе на C/C++ освобождение ресурсов превращается в танцы с бубном. Неужели на столько лет существование языков никому из разработчиков стандартов в голову не приходило как-то упростить этот постоянно использующийся процесс?
Существует примерно с рождения языка - RAII. А если нужно "сходить" в С, тогда, как написали выше, писать врапперы на это дело.
А это бессистемный подход какой то.
для начала надо понять, что нет такого зверя "C/C++"
после этого нормально выучить хотя бы основы C++, чтобы понять как легко и корректно управлять ресурсами
Ещё вариант, без мам, пап и unique_ptr
bool MakeDumpToFile(DWORD pid, PCWCHAR filename)
{
Library dbgdll("dbghelp.dll");
if (!dbgdll.Ok()) return false;
auto pfnMiniDumpWriteDump = (decltype(&MiniDumpWriteDump)) dbgdll.GetAddress("MiniDumpWriteDump");
if (!pfnMiniDumpWriteDump) return false;
Handler<HANDLE> proc(procOpenProcess(PROCESS_ALL_ACCESS, FALSE, pid));
if (!proc.Ok()) return false;
Handler<HANDLE> file(CreateFileW(filename, GENERIC_WRITE, NULL, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL));
if (!file.Ok() && file.Get() == INVALID_HANDLE_VALUE) return false;
bool result = pfnMiniDumpWriteDump(proc.Get(), pid, file.Get(), MiniDumpNormal, NULL, NULL, NULL);
return result;
}
А откуда этот Handler взялся?
Примерно так, так как HANDLE используется чуть менее чем во всем WinAPI облегчит жизнь. Это общая идея. Таким образом типизировать другие типы. Добавить inline, но со сборкой O2 думаю и так компилятор догадается.
template <typename T>
class Handler
{
public:
Handler(T handle) :
_handle(handle)
{
}
~Handler()
{
if (_handle != nullptr) CloseHandle(_handle);
}
T Get()
{
return _handle;
}
bool Ok()
{
return _handle != nullptr;
}
private:
T _handle;
};Предлагаю не изобретать того что написано невероятное количество раз и использовать готовое и продуманное
https://github.com/kelbon/logic_guards
Вашему решению не хватает лаконичности и гибкости. Все те же задачи решаются простым defer из данной статьи, без введения лишних сущностей. Если вам нужен какой-то условный defer, вы можете просто явно прописывать эти условия в самой отложенной лямбде. Это мало того, что требует меньше кода, так ещё и более гибко: можно одной переменной отменять сразу несколько отложенных лямбд, в отличие от вашего решения.
Предложенный defer имеет очень простую концепцию. Его легко воспроизвести (всего десяток строк кода), легко усвоить (всего одно новое ключевое слово), и просто адаптировать прямо на месте под нужды вашей функции, добавляя любые условия как на месте вызова defer (путём захвата cond по значению), так и на месте выполнения лямбды (читая значения переменных по ссылкам).
Если вы хотите подробного разбора, ну давайте. У меня scope_exit занимает буквально 3 строки, всё остальное сделано для удобства использования и максимальной эффективности.
scope_failure сделано отдельным как раз чтобы не писать самому эти булевы флаги, а в удобной и читаемой форме описывать свои действия
Не смотря на кажущееся сходство и простоту обоих решений (моего и вашего), у них есть серьёзные различия:
ваш defer:
не компилируется в constexpr функции: https://godbolt.org/z/o84P1fGMT
не является empty типом, если лямбда внутри empty, это важно если вы решите вдруг сохранить это как поле типа или, что важнее, неявно компилятор сохранит скоп екзит на фрейме корутины
не является агрегатом, так как имеет конструктор. Это в свою очередь приводит к тому, что у вас теряется агрегатная инициализация, а в случае вашего конструктора вообще появляется копирование. Значит если вы всё таки решите захватывать в лямбду что-то кроме ссылок (вы вроде бы специально под это сделали макрос), то у вас будет лишнее копирование, в лучшем случае лишний мув. В моей реализации не будет ни мува, ни копирования.
Более того, ваш скоп екзит игнорирует исключение из копирования при конструировании. То есть скоп екзит не гарантирует, что функция всё таки будет вызвана, а именно это он хотел гарантировать
игнорируется исключение при вызове в деструкторе
Все эти проблемы в моём решении учтены
В scope_failure вдаваться не буду, как там лендинги расставляются и почему оно так написано
Ваше решение содержит недочеты и ошибки в реализации, исправление которых приведет к реализации @Kelbon.
Сколько пафоса ) А вот с английским языком в описании надо поработать...
Что будет, если f бросит исключение?
Макросы - зло.
Что будет, если f бросит исключение?
То же самое, что и в случае исключения в любом деструкторе. Не стоит бросать исключения в деструкторах =)
Макросы - зло.
Это правда, но небольшая щепотка, которая облегчает жизнь, вполне допустима. Тем более в C-подобном коде, где ими и так щедро усыпано.
Ваш макрос defer конфликтует с Boost.Asio.
К сожалению, там используется шаблон под названием defer, поэтому макрос с таким же именем по понятным причинам несовместим с этим кодом. Можете попробовать заменить имя макроса на DEFER или что-то другое, что не пересекается с вашим кодом.
Как указано в первом абзаце статьи, defer хорош для низкоуровневого си-подобного кода. Boost в таком случае встречается редко, обычно нет поддержки исключений, часто нет полноценного STL, а иногда даже и стандартной библиотеки C. Если же у вас в проекте используются высокоуровневые библиотеки типа Boost.Asio, то наверное стоит и для всего остального использовать полноценные RAII-врапперы, либо же разделять высокоуровневый и низкоуровневый код по разным translation unit.
Вы только что scope_exit
std::unique_ptr<std::remove_pointer_t<HANDLE>,decltype(&CloseHandle)>
Именно для этого и придуманы алиасы и авто. Ну и как предложили выше можно сделать нормальную обёртку конкретно для виндовых структур.
Мне не нравится вариант с unique_ptr тем, что будет по крайней мере один new. Банальный пример, а уже лезем в память. Возможно, что как то все оптимизируется. Я не вникал, как оно там устроено. Если, что поправьте.
Конечно, не будет! unique_ptr - RAII обертка над самым обычным указателем, и никаких new в тайне от пользователя не делает.
Скорее всего, вы путаете сам unique_ptr с библиотечной функцией std::make_unique, которая является просто сокращенной записью вот этого:unique_ptr<T>(new T(std::forward<Args>(args)...))
Классический goto cleanup на чистом C вроде не так пишут?
Обычно он выглядит приблизительно так:
bool MakeDumpToFile(DWORD pid, PCWCHAR filename)
{
bool result = false;
HMODULE dbgdll = NULL;
decltype(&MiniDumpWriteDump) pfnMiniDumpWriteDump = nullptr;
HANDLE proc = NULL;
HANDLE file = NULL;
dbgdll = LoadLibraryA("dbghelp.dll");
if (!dbgdll) { goto loadlib_failed; }
pfnMiniDumpWriteDump = (decltype(&MiniDumpWriteDump)) GetProcAddress(dbgdll, "MiniDumpWriteDump");
if (!pfnMiniDumpWriteDump) { goto getprocaddr_failed; }
proc = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid);
if (!proc) { goto openprocess_failed; }
file = CreateFileW(filename, GENERIC_WRITE, NULL, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
if (!file || file == INVALID_HANDLE_VALUE) { goto createfile_failed; }
result = pfnMiniDumpWriteDump(proc, pid, file, MiniDumpNormal, NULL, NULL, NULL);
createfile_failed:
CloseHandle(file);
openprocess_failed:
CloseHandle(proc);
getprocaddr_failed:
FreeLibrary(dbgdll);
loadlib_failed:
return result;
}
У варианта со вкусом unique_ptr:
std::unique_ptr<std::remove_pointer_t<HANDLE>, decltype(&CloseHandle)> proc(OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid), &CloseHandle);
if (!proc) { return false; }
есть то преимущество, что нет шансов добавить какой-то код между получением хэндла и созданием очищающего объекта для этого хэндла. У нас либо есть proc, для которой будет вызван деструктор, либо нет.
Тогда как в варианте с defer:
HANDLE proc = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid);
if (!proc) { return false; }
defer [&] { CloseHandle(proc); };
Запросто можно добавить еще один if перед defer и получить приключения:
HANDLE proc = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid);
if (!proc) { return false; }
if (some_phase_of_the_moon) { return false; }
defer [&] { CloseHandle(proc); };
И попробуй такую потенциальную утечку ресурсов затем найти.
ЗЫ. unique_ptr далеко не всегда подходит для очистки ресуров, для таких целей можно сделать небольшую универсальную обертку над разнотипными хэндлами. Вот здесь я когда-то давным-давно показывал пример класса handle_holder.
ЗЗЫ. Ув.тов.Kelbon правильно в своем первом комментарии указал про копирование лямбды. Это серьезный просчет.
wrl/comptr
com-coding-practices
так еще можно, наверно
Как видите, решение настолько простое и прямо напрашивается, что другой человек написал бы почти то же самое, просто имея похожую идею в голове =)
Впрочем, перед тем, как писать свой вариант (несколько лет назад), я немного гуглил, и мне почему-то попадались примеры оверинжиниринга. Кто-то даже использовал std::function для хранения лямбды (что совершенно не бесплатно и генерирует кучу ненужного в данном случае кода).
Не совсем понимаю в чем реализация полагается на C++17, тут C++11 должно быть достаточно.
И ещё можно улучшить, затащив [&] внутрь макроса, тогда можно будет писать почти как в Go:defer { close(file); };
И ещё можно улучшить, затащив [&] внутрь макроса
Об этом написано в самом конце статьи. С явным [&] выше гибкость (например, можно организовать условный defer, для которого необходимость выполнения вычисляется на месте defer), лучше наглядность (сразу понятно, что это лямбда, и return какого-то значения из неё никак не повлияет на результат внешней функции), и точка с запятой в конце имеет смысл (так как это defer callable;, а не некое подобие конструкции языка вроде if {}, где точка с запятой в конце не требуется). Ну и как вишенка на торте, когда я изначально делал этот макрос, был актуален пропозал добавить defer в Си, и он использовал именно такой синтаксис (но его, увы, не приняли, хотя как встроенная фича в Си он очень был бы кстати).
Не совсем понимаю в чем реализация полагается на C++17, тут C++11 должно быть достаточно.
Пришлось бы усложнять код, иначе не скомпилируется. В C++17 подтянули автовывод типов в шаблонах, плюс добавили гарантированный copy elision в некоторых случаях типа A a = A();. Кажется, с более старыми стандартами я не сталкивался уже более 5 лет. Нет смысла писать более сложный код под устаревшие компиляторы только потому, что это в принципе возможно.
Если бы C++23 был уже в мейнстриме, я бы сразу в него и целился. Там вместо громоздкого:
#define TOKEN_CONCAT_NX(a, b) a ## b
#define TOKEN_CONCAT(a, b) TOKEN_CONCAT_NX(a, b)
#define defer deferrer TOKEN_CONCAT(__deferred, __COUNTER__) =
можно было бы использовать недавно стандартизированный плейсхолдер _ и написать просто:
#define defer deferrer _ =
Через несколько лет так и будем писать =)
У меня аналогичный defer уже как много лет на 11 стандарте работал на многих моих проектах. Условный дефер не нужен, так как условие можно внутри блока написать. Поэтому не вижу смысла показывать что это лямбда. Семантика defer и так максмально прозрачна и понятна.
люди стандартизируют вещи, чтобы через несколько лет вообще перестать писать макросы
На прошлом проекте использовали для отработки действий при выходе из области видимости BOOST_SCOPE_EXIT_ALL. На текущем gsl::finally. Но тащить целую библиотеку для этого не всегда есть смысл, конечно.
Всё таки я не понимаю, чем это принципиально отличается от записи deferrer name([&]{...}); ? Неужели так трудно придумать имя? Зато можно добавить в темплейтный класс доп. функционал вроде условного выполнения (как у @Kelbonвыше) и не нужно загаживать код макросом.
спасибо за идею как написать такую конструкцию ещё лаконичнее!
замечание: по крайней мере под Windows есть макрос _CONCAT(x, y), который делает то же самое что и TOKEN_CONCAT(a, b) у вас
Guideline Support Library (GSL)
Авторы: Бьярне Страуструп, Херб Саттер, Нил Макалистер.
Некоторые идеи уже перекочевали в стандарт C++ (std::span (C++20) - аналог gsl::span, std::byte (C++17) - похожая идеология с gsl::byte).
Реализация gsl::finally у MS (GSL/include/gsl/util at main · microsoft/GSL · GitHub) - 28 строк. Пример использования.
#include <gsl/gsl>
void example() {
auto cleanup = gsl::finally([] {
// This will be called when the scope ends
std::cout << "Cleanup executed\n";
});
// ... do work ...
// cleanup will be called automatically here
}
Лаконичный макрос defer для C++17