Comments 49
Движок habr для кода умный там, где не надо :)
Он схлопывает две пустые строчки в одну, поэтому не удаётся показать лишнюю пустую строчку в коде, про которою идёт речь. Она здесь:
Как вариант можно у нас в блоге посмотреть статью, там как показан так, как надо.
А я просто посчитал количество элементов в енуме и строковых констант, на 1 не сошлось.
Он схлопывает две пустые строчки в одну, поэтому не удаётся показать лишнюю пустую строчку в коде, про которою идёт речь.
Даже в code? Оо
Да
@Boomburumа проверьте, плиз? В моем понимании тег code должен исключительно бережно относиться к содержимому, а тут он фактически ломает код.
Ну такое, всё же здесь пресловутая "ошибка в ДНК" скорее, а то что нашли - уже вторичное проявление. Безусловно если код разрабатывать таким образом то без тулов для всевозможных проверок и анализа будет очень больно.
Если уж константы определили то нужно было коненчо не лениться и строки расставить в соответствии с ними, а не комменты к ним приписывать, т.е. массив инициализировать вот таким немного унылым образом:
s_status_str[DBG_STATUS_OK] = "Operation completed successfully";
s_status_str[DBG_STATUS_APP_VERSION_NOT_SET] = "Debug application version wasn't set";
//...я не знаток C/C++ так что может есть уже какая-то более удобная конструкция (можно было вообще ключи текстовые имхо и мапу использовать) - но принцип такой - не надо константы и значения по двум разным файлам разносить и пытаться их вручную с дурацкими комментами матчить. это обязательно "бумкнет" :)
в джаве сделали удобно - можно в самом enum добавлять произвольные поля и пользоваться им как этакой нерасширяемой мапой.
Проблема с такой инициализацией - она выполняется в рантайме, а та - статически компилятором. Начиная с С99 (то есть почти везде) можно и статически, но про это мало кто знает и редко использует.
https://www.geeksforgeeks.org/designated-initializers-c/
но про это мало кто знает
спасибо, огонь :) очевидно "красивая ошибка" настолько стара что ещё в C99 против неё попытались обезопаситься
Всегда так делаю. Кстати, забавно, что многие программисты C++ не знают о таком способе инициализации массивов.
А они должны?
Aggregate initialization - cppreference.com
Note: out-of-order designated initialization, nested designated initialization, mixing of designated initializers and regular initializers, and designated initialization of arrays are all supported in the C programming language, but are not allowed in C++.
я не знаток C/C++ так что может есть уже какая-то более удобная конструкция (можно было вообще ключи текстовые имхо и мапу использовать) - но принцип такой - не надо константы и значения по двум разным файлам разносить и пытаться их вручную с дурацкими комментами матчить. это обязательно "бумкнет" :)
Вот отправная точка:
https://habr.com/ru/articles/276763/
Взял идею, под свои потребности написал почти заново и нарадоваться не могу.
Или рефлексия по enum, или макросами такое делают.
На ревью кода конечно такое можно поймать, я бы здесь попросил добавить static_assert что размер второго массива совпадает с MAX_DBG_STATUS.
можно использовать в qed_dbg_get_status_str switch на енам dbg_status . В этом случае массив строк s_status_str уже не нужен. А компилятор ругнется если не все кейсы будут обработаны.
Да тут сам массив строк выглядит ужасно.
static const char * const s_status_str[] = {
[DBG_STATUS_OK] = "Operation completed successfully",
[DBG_STATUS_APP_VERSION_NOT_SET] = "Debug application version wasn't set",
[DBG_STATUS_UNSUPPORTED_APP_VERSION] = "Unsupported debug application version",
И все.
Поможет гарантировать, что к правильным индексами привязаны правильные строки, но не гарантирует, что для всех индексов прописаны строки.
Причем, если к последней определённой константе строка есть -- пропущенные в середине countof() не поймает.
не гарантирует, что для всех индексов прописаны строки.
А это толком и не отследить. Разве что в рантайме пройтись циклом и проверить длину каждой строки. В любом случае это куда менее опасная ошибка, чем сдвиг вообще всех индексов.
Пример по сути демонстрирует, что используя плохие практики программирования, можно получить трудноуловимые ошибки. Ну так не используйте плохие практики.
А это толком и не отследить
Есть много способов отследить и убедиться в этом. Как правило, игнорируемые по причине того, что код -- одноразовый. Написал и забыл, редактируется настолько редко, что нагораживание защит вокруг дороже обойдётся.
Лучший способ в данном конкретном случае (и всех аналогичных) -- использовать статический анализатор, который может поймать косвенно (как в статье) по использованию, или попытаться "догадаться" что массивы должны быть 1-в-1 с enum'ом.
Я удивлён, кстати ( @Andrey2008 ), что PVS-Studio не догадывается что это дескриптор для ENUMа и не проверяет на равенство элементов и порядка констант унутре. А ведь косяк в таких массивах часто получается из-за потерянной запятой.
Хм.. Место для подумать, спасибо.
Есть много способов отследить и убедиться в этом.
Средствами языка и без накладных расходов? Что-то ничего в голову не приходит.
Средствами языка и без накладных расходов:
Когда в массиве элементы перечислены последовательно -- сравнивая countof() в compile-time, мой основной подход;
Мета-макросы, как @RR_Zz упоминул ниже;
unit-testing с перечислением всех вариантов (я считаю форменным издевательством для данного случая, впрочем).
Средствами языка с накладными расходами:
Используя enum class + функцию с case вместо массива;
Проверки assert()'ы или просто if()'s в функциях доступа для проверок или включая проверки на доступ вне массива
Средставми вне языка без накладных расходов в рантайме:
стандартные статические анализаторы;
генератор кода по внешнему источнику (в каком-то смысле вариация x-macro)
ручной валидатор кода заточенный под конкретный случай, например, автогенерация юнит теста.
Это навскидку
Когда в массиве элементы перечислены последовательно -- сравнивая countof()
Что такое countof? gcc ругается, гугл ссылается куда-то в С++. Или я что-то неправильно понял?
Мета-макросы, как @RR_Zz упоминул ниже;
Интересная штука. Надо будет над ними помедитировать поподробнее.
unit-testing
Это все же не средствами языка
Спасибо. Вариант с мета-макросами интересный, может и пригодится когда.
sizeof -- размер в байтах, countof -- размер в элементах. Существует как _countof и в вариациях имени типа array_size, ARRAY_SIZE, std::ranges::size и все прочие. Просто проверяем, на момент компиляции, что размер массива сопадает числу элементов ENUMа.
Существует как _countof и в вариациях имени типа array_size, ARRAY_SIZE, std::ranges::size
На все эти варианты gcc ругается undefined reference to `countof'. Может, в Си этой функции все же нет? Или имеется в виду вообще любая, в том числе самописная, функция / макрос вида (sizeof(x)/sizeof(x[0]))?
Просто проверяем, на момент компиляции, что размер массива сопадает числу элементов ENUMа.
Если используется явное указание индексов {[x]=y, [z]=w,...}, это не спасет от пропущенного элемента в середине. А если как в исходном примере {y, w, ...}, то от перепутанного порядка.
Имеется ввиду любая реализация проверки на число элементов.
Да, как, и писал выше, явные индексы делают эту проверку бесполезной. В остальном случае, проверка достаточна с практической точки зрения для напоминания о месте использования. Типичные случаи добавления нового элемента проблем не доставляют, и пропущенную запятую при первоначальном наполнении ловит.
А это толком и не отследить
В Kotlin это отслеживается when(enumValue)
Тут можно последнюю константу потерять, и будет выход за границу массива.
Стоит ещё добавить размер:
static const char * const s_status_str[MAX_DBG_STATUS] = {
Этот код хорошо покрывается юнит тестами.
Но не покрыт :)
P.S. Про юнит-тесты в DPDK отдельная земетка скоро будет :)
А вот и обещанная статья "Поиск ошибок в юнит-тестах".
Тут прям напрашивается xmacro паттерн..
Тут вам любители Перла привет передают. Строго говоря это не ошибка, на корректность компиляции не влияет. Но вот на чтение людьми и последующие модификации, на GREP'ы и так далее - очень даже.
/* DBG_STATUS_DATA_DID_NOT_TRIGGER */
DBG_STATUS_DATA_DIDNT_TRIGGER,
Хорошо бы такие опечатки тоже ловить.
я нашел это только пропарсив оба файла перлом, загнав имена в словари и распечатав отсортированые ключи. ну и сравнив потом диффом.
Вот только найти её, просматривая код, ой как непросто.
Наивный вопрос не-программиста: анахрена так вообще делать, и как потом редактировать эти два отдельных списка имен и строк? Почему не писать что-то вроде:
const char STATUS_OK "Все в порядке" //у нас все получилось
const char STATUS_NOT_OK "Все не в порядке" //у нас ничего не получилось
const char STATUS_OOPS "Вообще все плохо" //Наташа, мы все уронилиПотому что все эти DBG_STATUS_ предназначены в первую очередь не для преобразования в строку, а чтобы отслеживать ошибки внутри программы. Возвращать из функций, использовать в качестве кода завершения программы, возможно для логов.
Идея использовать вместо короткого числа указатель на строку, конечно, интересная, но указатели ведь при каждом запуске новые будут. То есть когда программа упала, расшифровать коды ошибок будет сложно.
Так, что ли?
const char *STATUS_OK = "Все в порядке";Задача преобразования enum в строку решена весьма опасным способом. Лучше было бы написать switch и case под каждое значение. Современные компиляторы достаточно умны и укажут, если какое-то значение пропущено, главное - не ставить default. При этом этот код столь же оптимален, как и вручную заполненная таблица - компиляторы умеют switch в таблицу переходов преобразовывать.
https://godbolt.org/z/Gfo1b7G4e - как видно, что gcc, что clang умеют так оптимизировать код.
А я недавно ознакомился с вот такой особенностью С++.
// Объявление с именем example
char const* example() {
return "function";
}
// Имя example уже занято, но ошибок компиляции нет
// Объявление с именем example в struct namespace
struct example {
operator char const* () {
return "struct";
}
};
int main() {
char const* s = example();
// Выведет "function"
// Но если удалить функцию, выведет "struct"
std::cout << s << std::endl;
char const* ss = struct example();
// Выведет "struct"
std::cout << ss << std::endl;
}Объявление struct, class, union и enum работает так, словно неявно обернуто в typedef. Но только если имя типа еще не занято чем-то другим. Итого, получается потенциал для сложноотслеживаемого бага.
а можно было убрать массив и написать switch, где компилятор необработанные кейсы сам покажет
Продолжаем наслаждаться. DPDK: 100 больших и маленьких багов.
Самая красивая ошибка, которую я нашёл с помощью PVS-Studio в 2024 году