
![]() |
Наши проекты:
Журнал · Discuz!ML · Wiki · DRKB · Помощь проекту |
|
ПРАВИЛА | FAQ | Помощь | Поиск | Участники | Календарь | Избранное | RSS |
[18.219.250.4] |
![]() |
|
Страницы: (32) « Первая ... 29 30 [31] 32 ( Перейти к последнему сообщению ) |
Сообщ.
#451
,
|
|
|
Возможно, однако самую идею иллюстрирует. |
![]() |
Сообщ.
#452
,
|
|
Цитата JoeUser @ При чем тут вообще исключения? Вопрос в другом. В случае успешного прохождения полной цепочки инициализаций - ничего освобождать не нужно. После выхода из процедуры отдельные ресурсы должны оставаться валидными. Но, если не вся цепочка пройдена - вот тогда все ресурсы, захваченные до ошибки, должны быть освобождены. Чем моё решение не устроило? |
Сообщ.
#453
,
|
|
|
Цитата OpenGL @ Чем моё решение не устроило? Ну я уже там выше Киле писал о дроблении класса sftp_class ... Ты реализовал вот так: ![]() ![]() // ... // Сохранение локальных ресурсов в переменные класса session = std::move(local_session); socket = std::move(local_socket); ssh_init = std::move(local_ssh); // ... Ну уж если решил дробить, будь добр выделять не более чем по одному ресурсу на владельца. А их вот сколько вырисовывается: Цитата JoeUser @ 1) winsock_class - WSAStartup /WSACleanup(); 2) libssh2_class - libssh2_init / libssh2_exit 3) resolv_slass - getaddrinfo / freeaddrinfo <----------------- можно не хранить, т.к. резольвится один первый раз 4) socket_class - socket / closesocket 5) connect_class - connect / shutdown 6) libssh2_session_init_class - libssh2_session_init / libssh2_session_free 7) libssh2_sftp_sesstion_class - libssh2_sftp_init / libssh2_sftp_shutdown |
![]() |
Сообщ.
#454
,
|
|
Та на. Почти не отличается от оригинала
![]() ![]() ![]() #include <string> #include <vector> #include <chrono> #include <algorithm> #include <memory> #include <iterator> #include <filesystem> #include <fstream> #include <sstream> #include <cstdio> #include <cstdlib> // Windows stuff #include <io.h> // Linux stuff #include <sys/types.h> /*#include <sys/stat.h>*/ #include <fcntl.h> /*#include <unistd.h>*/ std::string load1(const std::string& path) { std::string buf; buf.reserve(std::filesystem::file_size(std::filesystem::path(path))); std::ifstream file(path); std::copy((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>(), std::back_inserter(buf)); return buf; } std::string load2(const std::string& path) { auto ss = std::ostringstream(std::string(std::filesystem::file_size(std::filesystem::path(path)), '\1')); std::ifstream file(path); ss << file.rdbuf(); return ss.str(); } std::string load3(const std::string& path) { auto close_file = [](FILE* f){fclose(f);}; auto holder = std::unique_ptr<FILE, decltype(close_file)>(fopen(path.c_str(), "rb"), close_file); if (!holder) return ""; FILE* f = holder.get(); if (fseek(f, 0, SEEK_END) < 0) return ""; const long size = ftell(f); if (size < 0) return ""; if (fseek(f, 0, SEEK_SET) < 0) return ""; std::string res; res.resize(size, '\1'); // C++17 defines .data() which returns a non-const pointer for (int i = 0; i < size; ++i) fread(&res[i], 1, 1, f); return res; } std::string load4(const std::string& path) { int fd = open(path.c_str(), _O_RDONLY); if (fd < 0) return ""; struct stat sb; fstat(fd, &sb); std::string res; res.resize(sb.st_size, '\1'); // C++17 defines .data() which returns a non-const pointer for (int i = 0; i < sb.st_size; ++i) read(fd, &res[i], 1); close(fd); return res; } // -------------------------------------------------------------------------------- class Test { const std::string path; const int k = 10; uint64_t ref_time = 0; public: Test(const std::string& p) : path(p) {} void run() { measure("C++ istreambuf_iterator", load1); measure("C++ stream::rdbuf", load2); measure("libc fread", load3); measure("POSIX read", load4); } private: template <typename FUNCTION> void measure(const char* name, FUNCTION load_function) { auto load = [this, &load_function]() {return load_function(path);}; printf("%-30s: ", name); fflush(stdout); uint64_t time = -1; for (int i=0; i < k; i++) { time = std::min(time, measure(load)); putchar('.'); fflush(stdout); } printf(" %10llu us", time); if (ref_time == 0) ref_time = time; else { printf(" (%0.2f)", ref_time/double(time)); } putchar('\n'); } template <typename FUNCTION, typename time_type = std::chrono::microseconds> uint64_t measure(FUNCTION fun) { const auto ts = std::chrono::high_resolution_clock::now(); const auto s = fun(); const auto te = std::chrono::high_resolution_clock::now(); static volatile size_t size = s.size(); return std::chrono::duration_cast<time_type>(te - ts).count(); } }; int main() { for (int i=1; i <= 32; i<<=1) { const std::string path = "test" + std::to_string(i) + ".fil"; std::ofstream(path.c_str(), std::ios::binary).write(&*std::vector<char>(i*1024*1024).begin(), i*1024*1024); printf("File %s\n", path.c_str()); Test test(path); test.run(); } return 0; } |
Сообщ.
#455
,
|
|
|
Цитата Qraizer @ Та на. Почти не отличается от оригинала С посимвольным чтением на libc и POSIX ты конечно отмочил! ![]() Тест же должен же был выявить максимальные скорости чтения. А ты им такую подляну кинул) Лучше бы два первых теста как-то забустил, типа: ![]() ![]() char Buffalo[64*1024*1024]; ... ... std::string load1(const std::string& path) { std::string buf; buf.reserve(std::filesystem::file_size(std::filesystem::path(path))); std::ifstream file(path); file.rdbuf()->pubsetbuf(Buffalo, 64*1024*1024); // <------------------- ... ... ... |
![]() |
Сообщ.
#456
,
|
|
Цитата JoeUser @ Ненуачё. Ему можно тролить, мне нельзя, что ль? С посимвольным чтением на libc и POSIX ты конечно отмочил! ![]() ![]() Добавлено И это ещё как посмотреть, кто тролил, а кто нет. Формально только rdbuf() по определению читает всё скопом, хоть и всё рано не знает, сколько там. А вот istream_iterator совместно с back_insert уже на подобное не способны, и то, что ты видишь, является оптимизацией уровня библиотеки. Так что получите и распишитесь – libc и ОС API наглядно демонстрируют возможности оптимизации: руками не оптимизируешь, хрен и получишь. Добавлено Та и никто в здравом уме не будет выбирать конкретный интерфейс и свято ему следовать "шаги по сторонам == расстрел". Нормальные пацаны берут все необходимые интерфейсы и юзают от каждого по потребности. Нужны блочные бинарные операции – std::ifstream::read(), нужны символьные форматирующие – std::basic_istream<>::operator>>(), понадобились файловые – std::filesystem, итп. Так что у него там по-любому синтетика, на практике бесполезная. |
![]() |
Сообщ.
#457
,
|
|
Цитата JoeUser @ Ну уж если решил дробить, будь добр выделять не более чем по одному ресурсу на владельца. Я не знаю, как лучше сделать конкретно в твоей программе. Если это имеет смысл (например, если это не единственное место, где нужны эти ресурсы), то выноси. А так я сделал только по числу меток cleanup и чисто для демонстрации того, как это сделает здоровый плюсовик, а не плюсовик курильщика. |
Сообщ.
#458
,
|
|
|
Цитата OpenGL @ (например, если это не единственное место, где нужны эти ресурсы) Вооо, вернулись к началу начал ![]() |
Сообщ.
#459
,
|
|
|
Цитата JoeUser @ Если добрались, все эти ресурсы в полях класса висят без какого-то использования, только для удаления. Так ведь именно так и будет в том случае, который тебе предлагают ![]() А если речь об указателях (т.е. ресурсы в виде указателей у тебя), то тебе не нужно писать классы-обертки. Достаточно unique_ptr и typedef/using Добавлено Тут ведь что важно. Код всяких load станет проще и чище. Да, за счет дополнительных утилитарных структурок/typedef. Добавлено Цитата JoeUser @ Поэтому - гвардов вполне достаточно, хоть и таких мудреных с if-фами. Если тебе нравится лапша в коде, то кто ж тебе помешает? ![]() Добавлено JoeUser, еще я понял, что есть разница в восприятии у нас. Ты как будто весь код целиком смотришь. Потому для тебя и нет разницы, где ресурс будет освобожден, в коде функции load или в отдельном объекте-владельце, даже второй случай кажется тебе более сложным. Я же предпочитаю декомпозировать. И мне важнее, что код load более простой и чистый, не замылен лапшой с очисткой ресурсов. |
Сообщ.
#460
,
|
|
|
Кстати, я иногда поражался способности некоторых людей быстро анализировать сложный запутанный код с большим количеством переходов и пр.
Это задача не для средних умов явно. А я в таком коде точно сделаю кучу багов и буду не способен поддерживать его уже через пару дней после написания. Так что для таких как я остается только декомпозировать, максимально упрощать, делать небольшие блоки, которые отвечают за что-то одно, максимально автоматизировать, писать юнит-тесты и т.д. и т.п. |
Сообщ.
#461
,
|
|
|
Цитата D_KEY @ А если речь об указателях (т.е. ресурсы в виде указателей у тебя), то тебе не нужно писать классы-обертки. Достаточно unique_ptr и typedef/using Ну у меня почти поровну разных. Первые два - только "факт" что промежуточная инициализация прошла (чтобы потом финализировать). Третий - нужно сразу удалить память после и не хранить. А вот остальные три - указатели на различные структуры. Цитата D_KEY @ Если тебе нравится лапша в коде, то кто ж тебе помешает? Лапша в коде - это генерация "автономных" RAII там, где это неуместно. Во всех мануалх и бест-практиках твердят: используйте инструментарий там, где это уместно. Так вот тут, я считаю, лапша - это искусственное разбиение единого процесса, только с одной целью - впилить туда всеми правдами и неправдами RAII. И лапша и костыли - вот это и есть такое навязывание. Короче. Возвращаюсь к goto. Ваши попытки - неубедительны ![]() |
Сообщ.
#462
,
|
|
|
Цитата JoeUser @ Лапша в коде - это генерация "автономных" RAII там, где это неуместно. Это не лапша, потому что это уже другой код, он к функции load не имеет отношения прямого. Можно отдельно писать (даже разными людьми) и затем поддерживать. Вот тебе нужно будет поменять что-то в коде load. Тебе придется заново обдумывать, не сломалось ли чего. Мне не придется. У меня логика работы с ресурсами отделена. Добавлено Цитата JoeUser @ Во всех мануалх и бест-практиках твердят: используйте инструментарий там, где это уместно. Да, RAII уместно использовать для работы с ресурсами. Цитата искусственное разбиение единого процесса Это не единый процесс, ИМХО. Цитата Короче. Возвращаюсь к goto. Ваши попытки - неубедительны ![]() Да кто ж тебе мешает-то? ![]() Можешь потом код класса скинуть? Может на RAII переведу, чтоб была полная картина для спора. |
Сообщ.
#463
,
|
|
|
Цитата JoeUser @ Можешь потом код класса скинуть? Может на RAII переведу, чтоб была полная картина для спора. Да не вопрос. Я может быть его и на гитхаб выкину (кстати, переписываю его без Qt), . Потому как норм библиотеки для SFTP нету. А платные я покупать не намерен! Вот там и форкнешь. Цитата D_KEY @ Это не лапша Нет, это лапша! ![]() Цитата D_KEY @ Это не единый процесс Нет, это единый процесс! ![]() Добавлено Цитата D_KEY @ Вот тебе нужно будет поменять что-то в коде load Для это изначально нужно тщательнее продумывать. И решать лучше проблемы по мере поступления. Раз пришлось потом что-то менять - значит это тебе урок: не ленись на начальном проектировании! |
Сообщ.
#464
,
|
|
|
Цитата JoeUser @ Если добрались, все эти ресурсы в полях класса висят без какого-то использования, только для удаления. Что тебе мешает объявить эти ресурсы не в полях класса, а там где конкретно они нужны? Тогда они сразу же и удаляться, как только перестанут быть нужными. В общем или лыжи не едут или я censored ![]() Тебе рассказывают про то, что хорошо бы работу с отдельными ресурсами вынести в отдельные классы, у тебя бы в таком случае код сократился бы в пять раз в твоей вырвиглазной функции, ты бы отделил логику работы приложения от логики работы с ресурсом, тем самым упростив понимание задачи. Тут работы на максимум на пару часов, и того меньше, а ты третьи сутки сидишь - и то у тебя не так, и это у тебя не эдак. Да тут даже вот взять если втупую не думая твой код скопипастить в класс, получается куда лучше и понятнее чем у тебя сейчас есть. Допустим на примере сессии: Какая то примитивная обертка(тупо взял все вызовы твоих функций и вынес в метод класса сессии): ![]() ![]() struct SLibssh2_session { SLibssh2_session() { m_session = libssh2_session_init(); if (!m_session) { sftpError = sftp_error::session_error; throw "session is not initialized..."; } } ~SLibssh2_session() { libssh2_session_disconnect(m_session, "Normal Shutdown, Thank you for playing!"); libssh2_session_free(m_session); } void SetBanner(const std::string& banner) { if(!libssh2_session_banner_set(m_session, banner.c_str())) { sftpError = sftp_error::session_error; throw "can't set baner..."; } } void Handshake(int socket) { if (libssh2_session_handshake(m_session, socket)) { sftpError = sftp_error::handshake_error; throw "handshake_error..."; } } void SetBlocking(int blocking) { // перевод в неблокируемый режим libssh2_session_set_blocking(m_session, blocking); } //! Эти методы возможно должны быть вынесены в другой класс, лень заморачиваться, просто ради показать идею. std::string HostKeyHash(int flag) { return libssh2_hostkey_hash(m_session, flag); } void LoginByUserPwd(const char* login, const char* pwd) { int ret = 0; while ((ret = libssh2_userauth_password(m_session, login, pwd)) == LIBSSH2_ERROR_EAGAIN); if (ret) { sftpError = sftp_error::auth_error; throw "error during LoginByUserPwd..."; } } void LoginByPublickey_frommemory(...) { while ((ret = libssh2_userauth_publickey_frommemory( m_session, Auth->login.toLocal8Bit().data(), Auth->login.length(), Auth->public_array.data(), Auth->public_array.size(), Auth->private_array.data(), Auth->private_array.size(), Auth->password.toLocal8Bit().data() )) == LIBSSH2_ERROR_EAGAIN); if (ret) { sftpError = sftp_error::auth_error; throw "Error in LoginByPublickey_frommemory..."; } } private: LIBSSH2_SESSION* m_session; } Использование, при этом логика твоего приложения никак не изменилась: ![]() ![]() void mov::qt::sftp_class::slotLogin() { struct addrinfo hint; struct addrinfo *addrs; struct sockaddr_in *sin; const char *fingerprint; struct addrinfo *p; int ret; bool found; sftpError = sftp_error::no; if (sftpState == sftp_state::ready && Auth->authState == auth_state::ok) { // инициализация библиотеки if (libssh2_init(0) != 0) { sftpError = sftp_error::libssh2_error; return; } // поиск адреса хоста memset(&hint, 0, sizeof(hint)); hint.ai_flags = AI_NUMERICHOST; hint.ai_family = AF_UNSPEC; hint.ai_socktype = SOCK_STREAM; hint.ai_protocol = IPPROTO_TCP; ret = getaddrinfo(Host.toLocal8Bit().data(), NULL, &hint, &addrs); if (ret == EAI_NONAME) { hint.ai_flags = 0; ret = getaddrinfo(Host.toLocal8Bit().data(), NULL, &hint, &addrs); } if (ret != 0) { sftpError = sftp_error::resolve_error; goto cleanup_libssh2_init; } found = false; for (p = addrs; p != nullptr; p = p->ai_next) { if (p->ai_family == AF_INET) { found = true; sin = reinterpret_cast<sockaddr_in *>(p->ai_addr); break; } } if (!found) { sftpError = sftp_error::ip4_error; goto cleanup_libssh2_init; } // создание сокета sock = socket(AF_INET, SOCK_STREAM, 0); if (sock == INVALID_SOCKET) { sftpError = sftp_error::socket_error; goto cleanup_libssh2_init; } sin->sin_family = AF_INET; sin->sin_port = htons(Port); // коннект ret = ::connect(sock, (struct sockaddr *)(sin), sizeof(struct sockaddr_in)); if (ret != 0) { sftpError = sftp_error::connect_error; goto cleanup_socket; } //======================Было============================================= // создание сессии // session = libssh2_session_init(); // if (!session) { // sftpError = sftp_error::session_error; // goto cleanup_socket; // } // // установка баннера // if (libssh2_session_banner_set(session, "SSH-2.0-OpenSSH_LIBSSH2_1.9.0") != 0) { // sftpError = sftp_error::session_error; // goto cleanup_session; // } // // рукопожатие // if (libssh2_session_handshake(session, sock)) { // sftpError = sftp_error::handshake_error; // goto cleanup_session; // } // // перевод в неблокируемый режим // libssh2_session_set_blocking(session, 0); // получение отпечатка //fingerprint = libssh2_hostkey_hash(session, LIBSSH2_HOSTKEY_HASH_SHA1); // авторизация // if (Auth->authType == auth_type::login) { // while ((ret = libssh2_userauth_password( // session, // Auth->login.toLocal8Bit().data(), // Auth->password.toLocal8Bit().data() // )) == LIBSSH2_ERROR_EAGAIN); // if (ret) { // sftpError = sftp_error::auth_error; // goto cleanup_session; // } // } else { // while ((ret = libssh2_userauth_publickey_frommemory( // session, // Auth->login.toLocal8Bit().data(), // Auth->login.length(), // Auth->public_array.data(), // Auth->public_array.size(), // Auth->private_array.data(), // Auth->private_array.size(), // Auth->password.toLocal8Bit().data() // )) == LIBSSH2_ERROR_EAGAIN); // if (ret) { // sftpError = sftp_error::auth_error; // goto cleanup_session; // } // } //=======================Стало============================================ try { SLibssh2_session session; session.SetBanner("SSH-2.0-OpenSSH_LIBSSH2_1.9.0"); session.Handshake(sock); session.SetBlocking(0); fingerprint = HostKeyHash(LIBSSH2_HOSTKEY_HASH_SHA1); if (Auth->authType == auth_type::login) { session.LoginByUserPwd(Auth->login.toLocal8Bit().data(), Auth->password.toLocal8Bit().data()); } else { session.LoginByPublickey_frommemory(...); } } catch(const char* error) { error; } //==================================================================== // инициализация ftp-сессии do { sftp_session = libssh2_sftp_init(session); if (!sftp_session) { if (libssh2_session_last_errno(session) == LIBSSH2_ERROR_EAGAIN) waitsocket(sock, session); else { sftpError = sftp_error::auth_error; goto cleanup_session; } } } while (!sftp_session); // все четко и дерзко sftpState = sftp_state::logged_in; return; // аварийная очистка // cleanup_session: // libssh2_session_disconnect(session, "Normal Shutdown, Thank you for playing!"); // libssh2_session_free(session); cleanup_socket: #ifdef WIN32 closesocket(sock); #else close(sock); #endif cleanup_libssh2_init: libssh2_exit(); } } Добавлено Разнеси оставшиеся ресурсы с очистками по классам, у тебя от функции останется 40 очевидных строк, которые будут понятны с полувзгляда. Обертки сами - они так же примитивные, по сути отдельные операции суешь в методы, выделение ресурса - конструктор, удаление - деструктор. Посмотри на получившийся класс - там сразу видно что это тупо обертка над Си функциями, ошибки детектятся мгновенно, т.к. каждая функция в отдельном методе, и если что можно сосредоточится на методе в 5 строк, нежели на функции в 200 строк вырвиглазного кода. Ошибку в 5 строках кода - найти проще, чем в 200 строках. Т.е. по сути класс ниочемный, но тем не менее - он логику работы с твоей сессией сократил в 3-4 раза. А если еще прикрутить логирование, в каждый метод, то тут как бы вообще даже если и будет какой то баг, то его без отладки найти будет дело трех минут. В твоем коде не то что бы отладка, тут без двух литров не обойтись чтоб понять где там что произошло. |
Сообщ.
#465
,
|
|
|
Цитата JoeUser @ Раз пришлось потом что-то менять - значит это тебе урок: не ленись на начальном проектировании! Нет ![]() Собственно, большая часть паттернов проектирования на это направлено, как и большая часть современных инженерных практик в области софта (юнит-тесты, *DD, CI/CD и пр.), да и принципов тоже (KISS, DRY, противостояние over engineering). Добавлено Цитата JoeUser @ Да не вопрос. Я может быть его и на гитхаб выкину (кстати, переписываю его без Qt), . Потому как норм библиотеки для SFTP нету. А платные я покупать не намерен! Вот там и форкнешь. ![]() А у тебя уже есть какие-то проекты выложенные? Я только собираюсь начать в эту сторону идти, есть пара идей на поиграться. Добавлено Цитата JoeUser @ Нет, это единый процесс! ![]() Я про отделение логики от возни с ресурсами. Добавлено Цитата Wound @ Тебе рассказывают про то, что хорошо бы работу с отдельными ресурсами вынести в отдельные классы, у тебя бы в таком случае код сократился бы в пять раз в твоей вырвиглазной функции, ты бы отделил логику работы приложения от логики работы с ресурсом, тем самым упростив понимание задачи. ![]() |