​​Результаты ревью
#опытным

Понимаю, что код был, мягко говоря, не для новичков, хоть и иллюстрировал начальный пример работы с epoll. Поэтому критиков не очень было много. Но все же Максим и Михаил хорошо постарались с поиском проблем. Давайте им похлопаем 👏👏👏.

А теперь суммируем.

Пройдемся по очевидному, что бросается в глаза сразу.

🔞 В функции read_data_and_count есть возвращаемое значение, но ничего не возвращается.

🔞 Повсюду утечки. Не освобождаются entries, не закрывается дескриптор epoll_fd.

🔞 Все три функции связаны и, если уж мы пишем на С++, то хочется все это дело обернуть в класс. А если не очень хочется, то хотя бы статиками пометить, чтобы скоуп не засорять.

🔞 Надоели указатели эти. Мы же С++, нам ссылки подавай!

🔞 Цикл, собирающий результаты мог бы быть более полюсовым и использовать стандартные функции из STL.

🔞 Как-то странно передавать десткрипторы именно массивом. Намного более универсальный инферфейс - указатели, так как массивы к ним неявно приводятся. А с указателями в вызывающем коде можно уже не сишный массив использовать, а нормальный вектор и передавать данные через метод data().

Из неочевидного:

😱 Использование VLA, то есть variable-length array, в строчке с epoll_event pending[N]. Это не стандарт языка С++, поэтому надо переделать на нормальный вектор.

😱 Не проверяются никакие ошибки. epoll_create1, epoll_wait, calloc, read - все эти системные функции не только говорят, что все летает и играет яркими красками, а еще и сигнализируют об ошибках. Здесь нигде их возвращаемые значения не проверяются, в надежде, что мы живем в мире розовых пони, где все всегда стабильно работает.

😱 Еполл может вернуть событие EPOLLERR в случае какой-то ошибки. А также он может прерваться из-за приема сигнала сгенерить ошибку EINTR. Опять же нет проверки.

😱 epoll_wait последним параметром принимает таймаут. -1 значит, что мы бесконечно блочимся в ожидании событий. И выходим из него только когда события произошли. Потенциально, это может длится примерно 100500 лет. Не очень хочется иметь потенциальную возможность получить бесконечный цикл. Поэтому какой-то таймаут поставить нужно, чтобы мы хотя бы об ошибке узнали.

😱 Не очень понятно, зачем обработчику событий знать о том, сколько файлов осталось, и влиять на это количество. Из process_epoll_event вполне можно возвращать индикатор, сигнализирующий о том, что мы прочитали данные из сокета до конца. Таким образом чисто внутренняя переменная files_left становится в единоличной власти своей материнской функции. Собственно, как и должно быть. Тогда и флаг done в структуре не нужен.

😱 Обо всех ошибках вызывающему коду хотелось бы знать, поэтому надо как-то сообщать ему о них. Можно по аналогии с сисколами использовать возвращаемое значение -1, как индикатор ошибки, но по плюсовому можно использовать std::optional. Ну или кидать исключения/возвращать объект ошибки, кому как нравится.

Фух, вроде все, что увидел, написал. Кто еще что-то заметит - не стесняйтесь, пишите в комменты. Исправленный код скину в комменты.

Let people critique your solutions. Stay cool.