покритикуйте код; slithercc

Ну и ещё я бы не делал вот таких глобальный данных:

std::deque<pkt_vec_t> pkt_queue;

Хз как соберут твою софтину, статическая libc или ещё какая магия, и деструктор pkt_queue будет дергаться после смерти libc.

Стандарт говорит:

3.6.3 Termination

Destructors (12.4) for initialized objects (that is, objects whose lifetime (3.8) has begun) with static storage duration are called as a result of returning from main and as a result of calling std::exit (18.5).

std::exit

https://en.cppreference.com/w/cpp/utility/program/exit

1. destructors of objects with static storage duration are called in reverse order of completion of their constructors or the completion of their dynamic initialization, and the functions passed to std::atexit are called in reverse order they are registered (last one first).

Т.е. деструктор глобальной переменной(static storage duration) вызовется когда надо.

Т.е. деструктор глобальной переменной(static storage duration) вызовется когда надо.

Процитированное говоришь лишь о том, что деструкторы будут вызваны в обратном порядке относительно завершения конструкторов. А порядок конструкторов гарантируется лишь в рамках одной единицы трансляции, а не между несколькими. Грубо говоря можно наступить в такое:

//1.cc
Dynamicly_initialized_object gv_obj;
//2.cc
extern Dynamicly_initialized_object gv_obj;
struct S {
   S() {gv_obj.fn();}
   ~S() {gv_obj.fn();}
}gv_s;

Если возьмешь за правило писать подобное или создавать какие-нибудь глобальные vector<S>, то есть вероятность вляпаться. И вот ты говоришь: «посмотрите код», а я вижу std::deque<pkt_vec_t> и мне надо напрягаться, смотреть что это за pkt_vec_t, а можно ли его так заюзать, лучше вообще так не делать и вопросов не будет. Аналогично с использованием new (который ты заюзал дважды), надо максимально заменять на make_unique() + unique_ptr. Исходники так проще смотреть грепом - убедился в отсутсвии new, грепнул *_cast, включил воринг Wold-style-cast. А не разбираться там в головоломках и «крутой» архитектуре, конечно, если ты вообще хочешь чтобы кто-то на код смотрел, а не ради пиара всё.

Да проще на Го или на Раст это дело переписать, чем так заморачиваться.

А что тут заморочительного то, я думал, что уже вообще никто приличный не пишет голый new в 95% случаях (placement new можно) и использует синглтоны.

Да, ЦПП тебе позволяет написать Г так как свобода, но есть и другая сторона у медали - этот говнокод видно за километр и, соответсвенно, уровень автора. Вот взять что-нибудь с гитхаба, мне достаточно нескольких минут, чтобы понять что за поделка передо мной и стоит ли туда лезть.

я вижу std::deque<pkt_vec_t> и мне надо напрягаться, смотреть что это за pkt_vec_t, а можно ли его так заюзать, лучше вообще так не делать и вопросов не будет.

А если сделать

static std::deque<pkt_vec_t> pkt_queue;

это избавит от неясности?

У тебя reinterpret_cast’ы кривые. […] в нужную структуру при получении в хендлере через std::memcpy

Согласен. Переделал на memcpy()

хоспади, какое счастье, что C++ прошел мимо меня, лишь легко задев.

и сам автор может забыть через пару лет

Да, это ключевой аргумент, который зачастую кажется неважным.

Если лицензия не указана, код считается проприетарным, и без явного разрешения автора ты ВООБЩЕ ничего не можешь с ним делать.

он выбирает только достойных, остальных подбирает пи**раст

Это кстати еще рмс в блоге рассуждал, что гитхаб никак не принуждает / не просит указывать лицензию, и поэтому гитхаб зло (ну, среди других причин).

Если бы не участвовал в форуме, то кого бы вы ругали?
Поэтому вы должны поблагодарить меня, принести мне челобитную и

ПОКЛОНИТЬСЯ ДО ПОЯСА

Шутка, повторенная дважды… Ну ты сам понял.

Благодарю всех за code review. Освежает. Правлю помаленьку.

почему в проекте используется куча сишного кода, всякие fread и т.п., он же на плюсах? Где умные указатели?

Умные закончились, остались только тупые

Я тут решил потестить на тебе свою поделку. Суть поделки такова - искать в коде бинарные операторы (* . % + - & ^ |), операнды которых имеют разную знаковость (signed/unsigned) и результат такого оператора кастится в результате в какой-то другой тип (как бы всегда так). Например:

int i = 0;
unsigned u = -1;
long res = i - u;

резуьтат здесь будет res == UINT_MAX, но скорее всего хотелось -1. Поэтому, на мой взгляд, такие места нужно находить и кастить один из операндов руками через static_cast, чтобы implicit cast не сломал ничего. Т.е. long res = i - static_cast<std::make_signed…>(u);

Вообще, имхо, это недоработка плюсов и нужно менять правила integer promotion, а пока так, буду юзать свои костыли. Подобных дефолтных ворингов нет, ближайший по смыслу Wsign-conversion, но он даёт много ложных сигналов, например operator= или implicit cast без бинарного оператора, в итоге получаешь портянку ворнингов разбираться в которых больно. Например, будет жалоба на получение с вектора элемента через интовый индекс, хотя очевидно, что она здесь не нужна. Обкладывать все подряд static_cast’ами - уродство и ревьюить больно.

В общем сам решай что с этой инфой делать, надо ли править, итог такой https://pastebin.com/kSYd2FVz

ЗЫ: а твои исходники скомпилить до конца не удалось, какие ошибки вываливаются (нет, это не ненайденные либы).

ЗЗЫ:

Поэтому, на мой взгляд, такие места нужно находить и кастить один из операндов руками через static_cast

Нет, лучше не через static_cast, а сделать небольшие функции tosig(), tousig() ибо опять же процитируя себя

Обкладывать все подряд static_cast’ами - уродство и ревьюить больно

ЗЫ: а твои исходники скомпилить до конца не удалось, какие ошибки вываливаются (нет, это не ненайденные либы).

Можно узнать операционку, компилятор, его версию, какие ошибки? Последний коммит:

commit cea08c1f40f76474917280729ee1bfdeba9c3524 
Date:   Mon May 10 15:26:00 2021 +0300

Я тут решил потестить на тебе свою поделку. Суть поделки такова - искать в коде бинарные операторы (* . % + - & ^ |), операнды которых имеют разную знаковость (signed/unsigned) и результат такого оператора кастится в результате в какой-то другой тип

Прикольная граблеискалка. Думаю надо починить сборку и прогнать ее еще раз чтобы она все проверила.

Можно узнать операционку, компилятор, его версию, какие ошибки?

linux

$ gcc -v
...
gcc version 10.2.0 (GCC)

На последнем коммите https://pastebin.com/cAcLa9qN

Прикольная граблеискалка. Думаю надо починить сборку и прогнать ее еще раз чтобы она все проверила.

Я её сюда залил

Нет, лучше не через static_cast, а сделать небольшие функции tosig(), tousig()

Не, тоже неверно. Должно быть правильней не допускать совсем в одном выражении справа от оператора= или compound= операндов с разными знаками, лишь что-то вроде:

sig = unsig
unsig += sig

ЗЫ: а твои исходники скомпилить до конца не удалось, какие ошибки вываливаются (нет, это не ненайденные либы).

Добавил сборку boost 1.67.0. Должно собраться.

Прикольная граблеискалка. Думаю надо починить сборку и прогнать ее еще раз чтобы она все проверила.

Я её сюда залил

Странно ругается:

game.cpp:229:20: warning: operands have different signs [-Wsign-arithmetic]

А там:

void game_t::pkt_food_set(const uint8_t* buf, size_t size)
[...]
	for (size_t idx = 0; idx < pkt_cnt; ++idx)
	{
		pkt_food_set_t pkt;
229:		memcpy(&pkt, buf + (sizeof(pkt) * idx), sizeof(pkt));

Там коммиты далеко уехали, сейчас это место уже не показывается. Если интересно, загляни в репозиторий. Там сейчас и подчеркивается всё, и место указывается. У тебя много кастов интовых литералов, т.е вместо 1U написано просто 1, например. На это можно забить (сконфигурировать плагин с -DSKIP_LITERALS=ON).

Там коммиты далеко уехали, сейчас это место уже не показывается

Это я натравил Wsign-arithmetic на коммит a625a1e82361f36f1918422b1486310fd4a6cb58 May 13 20:44:18 2021 +0300

Где умные указатели

Добавил.

почему в проекте используется куча сишного кода, всякие fread

Буду благодарен за C++ эквивалент кода:

#pragma pack(push, 1)
struct foo_t
{
    uint32_t foo;
    uint8_t bar;
};
#pragma pack(pop)

foo_t foo;
fread(&foo, 1, sizeof(foo), fh);

Там коммиты далеко уехали, сейчас это место уже не показывается. Если интересно, загляни в репозиторий. Там сейчас и подчеркивается всё, и место указывается. У тебя много кастов интовых литералов, т.е вместо 1U написано просто 1, например. На это можно забить (сконфигурировать плагин с -DSKIP_LITERALS=ON).

Обновил, пересобрал с -DSKIP_LITERALS=ON. Стало гораздо лучше видно. Напомнило вычистить старье из clock.h. Спасибо.