Pull to refresh

Comments 33

The exception will be generated in the case of memory allocation error.
Скажите пожалуйста, если вы знаете, что будет вызвано конкретное исключение, то почему вы просто не напишите какое? Мне кажется, разработчику будет проще вспомнить какой try...catch блок написать.
Вообще предложение попросту безграмотное. "An The exception will be generated in the case of memory allocation error"

Наймите нормального корректора, лучше всего носителя языка.

И вообще, не «generated », а «thrown»
Ну и вот это тоже по-русски английскими словами.

«There is no sense in testing the 'm_Data' pointer against null, as the memory was allocated using the 'new' operator»

По-английски нужно так

"There is no sense in Testing the 'm_Data' pointer against null makes no sense, as the memory was allocated using the operator 'new'"
А концовка не

"… using the 'new' operator"


должна быть?
Подождите, ведь the относится к оператору, которому нужен артикль.
Либо надо написать using 'new', либо как выше. Во всяком случае меня так учили.
Смотрите ответ ниже про «The boy Vasya»
Спасибо.
Однако, странно… Если просто ткнуть в ссылки на документацию от МС или Википедию, то там есть не только the new operator, но и the C++ language.
Даже книжка Бьярна называется the C++ programming language. Получается, что с точки зрения имени оператора — всё правильно.
Ну и

«Testing the 'm_Data' pointer ...»


Существительные же
То же самое, 'm_Data' — указывает на конкретый поинтер (извините за тавтологию). Вы же не говорите «The boy Vasya is playing with his ball»
Естественно, ведь правильно говорить «playing with his balls».
(Извините, не удержался =))
Можно конечно, хотя особенной надобности не вижу. А зачем ему вспоминать какой try..catch писать? Это неправильный подход. Тогда уж лучше старый добрый malloc() и вперёд. :)
malloc не любит инициализировать классы, поэтому для С++ его не стоит использовать. Для этого как раз и существует конструкция new(std::nothrow) MyClass(...)
Это понятно. Кстати, для случая new(std::nothrow) мы и не ругаемся. Я имею в виду, что код, подобный приведенному ниже, очень плох.

int *a = 0;
int *b = 0;
FILE *f = 0;
try
{
  int *a = new int[10];
  int *b = new int[10];
  f = fopen(...);
  ....
  delete [] a;
  a = 0;
  delete [] b;
  b = 0;
  fclose(f);
  f = 0;
}
catch (...)
{
  delete [] a;
  delete [] b;
  fclose(f);
}


Следует использовать умные указатели и т.п. Тогда никаких catch(...) не надо. Только где-то выше по уровню. Поэтому я и удивился, зачем здесь catch.
Скажите пожалуйста, а вы выдаёте предупреждения на строки вида?
int *a = 0;
Я понимаю, что это С++ и такое присваивание легально, но может быть лучше советовать либо NULL, либо nullptr?
Такой диагностики нет. И сомневаюсь, что появится. Подобные диагностики портят инструменты статического анализа. Анализатор выдаст очень много сообщений в некоторых проектах. И большинство программистов, будут считать их ложными (бессмысленными). Можно возразить, что непонравившуюся диагностику можно легко отключить. Однако, инструмент который выдает по умолчанию огромный поток бестолковых сообщений, не понравится на этапе знакомства. Программист смотрит в список сообщений, а там муть. Отключил одно сообщение, второе, а там всё муть и муть. Скорее всего, на этом знакомство и закончится.

Значит, делать такую диагностику в основном наборе правил никак нельзя. Хорошо, у нас есть «пользовательские» правила, которые по умолчанию выключены. Сейчас такими правилами являются: V2001-V2009 (см. документацию).

Но и туда, я бы не стал добавлять это правило. В силу огромного количества срабатываний в некоторых проектах, лог будет очень быстро расти. Сделаешь одно подобное правило, вроде не страшно. А сделаешь 5, и лог вдруг уже вырос 2 раза. А это замедление загрузки отчета, торможение при использовании фильтров для сообщений и так далее.
Сейчас по старой доброй традиции, обязательно будет вопрос, уведомили ли мы разработчиков. Ответ: Да.
Наверное стоило указать, что обнаружены ошибки в заголовках библиотечных именно, а не «Errors detected in C++Builder (PVS-Studio)».
Передам привет питерским ребятам из Embarcadero.
Там в Description:
We have checked the header files from the Embarcadero C++Builder XE3 project...
Помнится, пару лет назад в каком-то топике «Чего бы вы хотели добавить в PVS-Studio?» я описывал как раз случай «Неудачный макрос» и мне ответили, что фиг его знает, может и сделаем когда его детектирование. А ты ж смотри — действительно сделали!
It is possible that curly brackets are missing.

У меня просто нет слов. Не оборачивать макрос скобками — это детский сад.
> Обнаружилось достаточно много мест, где после вызова оператора 'new', проверяется, что указатель не равен NULL.
Эта вещь наверное не относится к стандарту и, возможно, отступает от его буквы — тяжёлое наследие. Однако у всех компиляторов C++, которые я видел, есть ключ «запретить использование исключений C++». Впрочем, современные компиляторы, точнее стандартная библиотека C++ поставляемая с компилятором, плюёт на этот ключ (она ведь скомпилирована с исключениями и по другому быть не должно), однако в истории каких только вариантов не было.

Вот с использованием этого ключа поведение может отличаться: начиная от честного предупреждения или ошибки, что для использования такой конструкции исключения должны быть разрешены, до тихой проверки на NULL (как в приведённом примере). Скорее поведение библиотечных функций при отключенных исключениях должно быть чётко документированно.
Допускаю. Но мне кажется, в большинстве случае всё проще. Местами поправили, а местами нет. Вот и всё. :)
Хотел бы я посмотреть на код PVS-Studio в плане архитектуры ) Феерическая сложность, наверное
Думаю, по сравнению со многими проектами, в PVS-Studio нет ничего сложного. Самой сложной частью, я бы пожалуй назвал механизм, который пытается угадать диапазон возможных значений в переменной.

Сложность анализатора в другом. Это огромное количество условий и паттернов, которые следует учитывать. По большой части, диагностические правила представляют собой леса запутанных if-ов. Идея диагностики часто проста и быстро программируется. А потом начинается настоящая работу по отсечению ложных срабатываний. Вот если так человек писал, то скорее всего не ошибка. И если тут число маленькое, то тоже не ошибка. А что такое маленькое число? А еще не ругаться, если это макрос. А если вот так, то наоборот уровень предупреждения поднять.

И начинается множество прогонов по базе open-source проектов, чтобы свести число ложных срабатываний к адекватному минимуму. Собственно, часто настройка правила отнимает в 10-20 раз больше времени, чем написание первоначального кода.

Конечно, из такого моего описания мало что понятно. Чтобы пояснить глубину омута, приведу комментарий, сделанный к диагностике V512. Эта полезная диагностика до сих пор дает много ложных срабатываний, хотя сделана масса исключений. Аналогичная ситуация со многими другими проверками.

/*
Генерирует предупреждение V512.

ДЛЯ ФУНКЦИЙ MEMCPY, MEMCHR, MEMCMP, MEMMOVE, MEMSET:

Опасным следует считать заполнение или копирование массива, если размер заполнения
не кратен размеру (или меньше размера) элементов массива.
Если указатель, получается при помощи операции взятия адреса & или это массив заданного размера,
то размер должен быть точно равен.

Что такое, AA, BB, CC:
Это аргументы с номером 1,2,3.
xxxx(AA, BB, CC)

ИСКЛЮЧЕНИЯ:

1) Если мы лезем в середину буфера, то скорее всего мы знаем что делаем и
   если находится underflow, то это не ошибка
   Пример: memcmp(p+7, "AA", 2);
2) Мы копируем в буфер (или сравниваем буфер) строку меньшего размера.
   Пример: char vendor[16]; memcpy(vendor, "Unknown", 8);
3) Мы копируем/сравниваем в буфер (но не memset и не memcmp) меньше чем его размер:
   sizeof(AA) < CC
   При этом размер данных определен как sizeof(простой тип/массив простых типов): CC = sizeof(простой тип).
   При этом размер второго аргумента, совпадает с заданным размером: sizeof(BB) == CC
   А адрес куда записывается (АА), не взят с помощью '&'. А то пропустим: memset(&size_t_var, 0, sizeof(int_var));
4) Мы делаем memset/memcpy/memmove, но на N байт меньше, чем размер буфера.
   А следующая строчками дозаписываем в конец какие-то значения.
   Признаки: в следующих строках есть AA, CC [+N], =.
   Из 'CC' может отсутствовать '*' и 'sizeof(...)'
5) У нас underflow и при этом в следующей строке вновь используется эта функция и тот-же буфер.
   memcpy(temp, status_flag, 2);
   memcpy(&temp[2], status, 2);
   Как вариант, в следующей строке к указателю прибавляется количество переданных байт.
   memcpy(temp, status_flag, 2);
   temp += 2;
6) Различные сравнения со строками. В целом можно сформулировать так.
   Безопасно сравнивать что-то с массивом типа char/wchar фиксированного размера, если
   мы сравниваем количество байт равное размеру строки (с учетом или без учёта терминального 0).
   Примеры:
   1) png_byte chunk_name[5]
      png_byte png_IDAT[5] = { 73,  68,  65,  84, '\0'}
      if (!png_memcmp(png_ptr->chunk_name, png_IDAT, 4))
   2) char fileStart[5];
      static const char header[] = "12345";
      if(!memcmp(header, fileStart, 5))
7) Третий аргумент содержит в себе название "BITMAPINFO" или "RGNDATA".
8) Это проверка производителя процессора: if (memcmp(&result._ebx, "GenuineIntel", 12) == 0)
   Признаки: сравниваем 12 байт и есть что-то из: AMDisbetter!, AuthenticAMD и т.д. (http://en.wikipedia.org/wiki/CPUID)
10) Размер массива равен 1/2 байта и первый/второй аргумент содержит в себе:
    .bmiColors, ->bmiColors, .FileSystemName, ->FileSystemName
11) Копирование данных в рамках одного буфера. memmove(a, a+1, x);
12) Если указатели вычисляются и все размеры не совпадают, то ошибки нет.
    Пример: memcpy (&(ProcessorName[4]), &(CPUExtendedIdentity[1]), sizeof (int));
13) Если проверяем структуру и нет переполнения. И сравниваем что-то с константной строкой.
    typedef struct {
      char ID[4];
      uint32 info;
    } UNIF_HEADER;
    if(memcmp(&unhead,"UNIF", 4)) 
14) Мы явно обрабатываем только первый элемент массива.
    int A[10]; if (!memcmp(&A[0], "1234", sizeof(int)));
15) Если мы ищем что-то в строке с помощью memchr, игнорируя последний 0.
16) Мы копируем в строку типа char/uchar несколько байт, а в следующей строке записываем терминальный 0.
    Пример: memcpy(text, P, 12);
            text[12] = '\0';
    Признаки: a) запись в строку типа char/uchar
              b) нет переполнения
              c) в следующей строке есть AA, CC, =, '\0'
17) Адрес буфера берется с помощью оператора & у переменной типа класс, у которого
    есть operator &.
18) Не связываемся со структурами, в конце которых есть массив из одного элемента.
    Пример: char buf[1];
    (Исключение реализовано с помощью информации, передаваемой в переменной oneElemArrayAtEnd).
19) Включается при заданном расширении "_UNDERFLOW_OFF"/"_OVERFLOW_OFF".
    Не считаем опасным underflow / overflow.
20) Джедайская работа с многомерными массивами вида: T A[4][4]; memcpy(&A[0][0], src, 16*sizeof(T));
Пример:
struct foo { 
  size_t a;
};
foo x;
memset(&x, 0, sizeof(unsigned));


ДЛЯ ФУНКЦИЙ STRCPY, WCSCPY, LSTRCPYA, LSTRCPYW, QSTRCPY, STRCAT

Опасно копировать/объединять строку в буфер меньшего размера.
При этом копируема строка явно представлена строковым литералом.

ИСКЛЮЧЕНИЯ:

1) Длина буфера равна 1. Такой прием используется при работе со структурами переменной длины.

ДЛЯ ФУНКЦИЙ STRNCPY, WCSNCPY

Опасно копировать строку в буфер меньшего размера, чем указано в count.
Пример:
    char szTarget[3];
    char *szState ="PVS";    
    strncpy(szTarget, szState, 3); 
*/

Не менее «ветвист» бывает у нас и код плагинов.

ЕСЛИ это плагин для VS2005 ИЛИ VS2005, ТО
...
ЕСЛИ для VS2010 ИЛИ 2012, ТО
...
ЕСЛИ для C++Builder XE3 UPDATE1, ТО
... а ЕСЛИ еще и включен 64-битный компилятор в C++Builder, ТО
....

ООП (полиморфизм) — не? Не для правил (понятно, что там слишком сложно), для плагинов.
Конечно все это есть и активно используется :-). Но любой полиморфизм в конечном итоге все-равно превращается в кучу «ЕСЛИ, ТО».
Не любой и не всегда. Если у вас просто разный код для 4-ех разных IDE, то это вполне себе укладывается в концепцию «один базовый класс с общими методами и 4 наследника с кастомным кодом для отдельных IDE». Конечно, «ЕСЛИ, ТО» в коде тоже будет, но уже не по поводу разделения функционала по IDE.
Макрос без фигурных скобок на пару с if убил насмерть. Они это тестом явно не покрыли. Вы ведь понимаете, что TVariant в результате от Currency* инициализируется с ошибкой для nullptr.
Громковатый стиль заголовка за «заметки». Искажает ожидания от содержимого.
Sign up to leave a comment.