Как правильно переписать код ASSERT для передачи / анализа в msvc? - PullRequest
Новая
       30

Как правильно переписать код ASSERT для передачи / анализа в msvc?

10 голосов
/ 09 апреля 2010

Visual Studio добавил анализ кода (/analyze) для C / C ++, чтобы помочь идентифицировать плохой код. Это довольно приятная функция, но когда вы работаете со старым проектом, вы можете быть поражены количеством предупреждений.

Большинство проблем возникает из-за того, что старый код выполняет команду ASSERT в начале метода или функции.

Я думаю, что это определение ASSERT, используемое в коде (из afx.h) 

#define ASSERT(f)          DEBUG_ONLY((void) ((f) || !::AfxAssertFailedLine(THIS_FILE, __LINE__) || (AfxDebugBreak(), 0)))

Пример кода: 

ASSERT(pBytes != NULL);
*pBytes = 0; // <- warning C6011: Dereferencing NULL pointer 'pBytes'

Я ищу простое, чистое и безопасное решение для устранения этих предупреждений, которое не подразумевает отключение этих предупреждений . Я упоминал, что в текущей кодовой базе много вхождений?

Ответы [ 6 ]

5 голосов
/ 09 апреля 2010

/analyze не гарантирует выдачу соответствующих и правильных предупреждений. Он может и пропустит множество проблем, а также дает ряд ложных срабатываний (вещи, которые он определяет как предупреждения, но которые совершенно безопасны и никогда не произойдут)

Нереально ожидать нулевого предупреждения с / анализировать.

Он указал на ситуацию, когда вы разыменовываете указатель, который it не может проверить, всегда действителен. Насколько PREfast может сказать, нет никакой гарантии, что он никогда не будет NULL.

Но это не значит, что может быть NULL. Просто анализ, необходимый для доказательства его безопасности, слишком сложен для PREfast.

Возможно, вы сможете использовать специфическое для Microsoft расширение __assume, чтобы сообщить компилятору, что он не должен выдавать это предупреждение, но лучшим решением будет оставить предупреждение. Каждый раз, когда вы компилируете / анализируете (что необязательно каждый раз, когда вы компилируете), вы должны убедиться, что предупреждения, которые он выдает, все еще являются ложными срабатываниями.

Если вы правильно используете свои утверждения (для обнаружения логической ошибки во время программирования, защиты от ситуаций, которые не могут произойти, то я не вижу проблем с вашим кодом или с выходом предупреждения. Добавление кода для обработки проблема в том, что может никогда не возникать, просто бессмысленна. Вы добавляете больше кода и усложняете без причины (если этого никогда не произойдет, то у вас нет способа восстановить его, потому что вы абсолютно не знаете в каком состоянии находится программа. Все, что вы знаете, это то, что она ввела путь кода, который вы считали невозможным.

Однако, если вы используете свой assert в качестве фактической обработки ошибок (значение может быть NULL в исключительных случаях, вы просто ожидаете, что этого не произойдет), то это дефект в вашем коде. Затем требуется правильная обработка ошибок (как правило, исключений).

Никогда не используйте подтверждения для проблем, которые возможны . Используйте их, чтобы убедиться, что Невозможно не происходит. И когда / анализ дает вам предупреждения, посмотрите на них. Если это ложное срабатывание, игнорируйте его (не подавляйте его, потому что, хотя это ложное срабатывание сегодня, код, который вы проверяете завтра, может превратить его в реальную проблему).

3 голосов
/ 27 сентября 2013

Во-первых, ваше заявление о подтверждении должно гарантировать прекращение или прекращение подачи заявления. После некоторых экспериментов, которые я обнаружил в этом случае / анализ игнорирует все реализации в шаблонных функциях, встроенных или обычных функциях. Вместо этого вы должны использовать макросы и трюк do {} while (0) со встроенным подавлением

Если вы посмотрите на определение ATLENSURE () Microsoft использует __analyse_assume () в своем макросе, у них также есть несколько параграфов очень хорошей документации о том, почему и как они переносят ATL для использования этого макроса.

В качестве примера я изменил макросы CPPUNIT_ASSERT таким же образом, чтобы очистить тысячи предупреждений в наших модульных тестах. 

#define CPPUNIT_ASSERT(condition)                                                                    \
do {    ( CPPUNIT_NS::Asserter::failIf( !(condition),                                                \
                                  CPPUNIT_NS::Message( "assertion failed" ),                         \
                                  CPPUNIT_SOURCELINE() ) ); __analysis_assume(!!(condition));        \
                                  __pragma( warning( push))                                          \
                                  __pragma( warning( disable: 4127 ))                                \
                                  } while(0)                                                         \
                                  __pragma( warning( pop))
3 голосов
/ 09 апреля 2010

PREFast сообщает вам, что у вас есть дефект в вашем коде; не игнорируйте это. У вас действительно есть один, но вы только пробежались вокруг, признавая это. Проблема заключается в следующем: просто потому, что pBytes никогда не был NULL в разработке и тестировании, не означает, что он не будет в производстве. Вы не справитесь с такой возможностью. PREfast знает об этом и пытается предупредить вас о том, что производственные среды враждебны, и оставит ваш код дымящей, искаженной массой бесполезных байтов.

/ декламация

Есть два способа исправить это: Правильный путь и взлом.

Правильный способ заключается в обработке указателей NULL во время выполнения: 

void DoIt(char* pBytes)
{
    assert(pBytes != NULL);
    if( !pBytes )
        return;
    *pBytes = 0;
}

Это заставит замолчать ПРЕДВАРИТЕЛЬНО.

Хак - использовать аннотацию. Например: 

void DoIt(char* pBytes)
{
    assert(pBytes != NULL);
    __analysis_assume( pBytes );
    *pBytes = 0;
}

РЕДАКТИРОВАНИЕ: Вот ссылка , описывающая ПРЕДВАРИТЕЛЬНЫЕ аннотации. В любом случае, отправная точка.

0 голосов
/ 10 апреля 2010

Мой соавтор Дэвид Лебланк сказал бы мне, что этот код все равно не работает, предполагая, что вы используете C ++, вы должны использовать ссылку, а не указатель, и ссылка не может быть NULL:)

0 голосов
/ 09 апреля 2010

помните, ASSERT () исчезает в розничной сборке, поэтому предупреждение C6011 абсолютно верно в приведенном выше коде: вы должны проверить, что pBytes не равен NULL, а также выполнить ASSERT (). ASSERT () просто выбрасывает ваше приложение в отладчик, если это условие выполняется при ошибке отладки.

Я много работаю над / анализирую и ПРЕДВАРИТЕЛЬНО, поэтому, если у вас есть другие вопросы, пожалуйста, дайте мне знать.

0 голосов
/ 09 апреля 2010

Вы, кажется, предполагаете, что ASSERT(ptr) как-то означает, что впоследствии ptr не равно NULL. Это не так, и анализатор кода не делает этого предположения.

...