О зле, случайно призванном учениками магов




Изучение языков программирования – дело долгое и трудоёмкое. Однако тернистый путь неизбежен, если планируешь основательно изучить язык, принципы, средства и особенности работы с ним. С++ тому не исключение, а наоборот - наглядный пример. Он содержит множество нюансов, которые необходимо знать и учитывать при работе с ним. Но, как уже отмечалось выше, для этого нужны время и практика.

Picture 1

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

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

Ближе к делу

Материалом к данной статье послужили файлы, размещённые на сайте Pastebin.com. Как правило, это файлы лабораторных работ, написанных разными студентами. И конечно в них есть ошибки. В этой статье рассмотрены некоторые интересные (в определённом смысле) участки кода. В общем, статья носит сразу 2 характера: развлекательный для опытных программистов и познавательный для новичков.

Инструментом для анализа являлся статический анализатор кода - PVS-Studio. Так что зачастую участки кода будут сопровождаться предупреждениями анализатора.

"Овсянка, сэр!"

Циклы, циклы, циклы...

От баснословных речей ближе к делу.

Давайте взглянем на код:

void stampa_triangolo_rettangolo (int n)
{
  for (int i=0; i<n, i++;)
  {
    for (int j=0; j<n, j++;)
    {
      if (j<i)
        cout<<"  ";
      else
        cout<<"* ";
    }
    cout<<endl;
  }
}

Сообщение анализатора: V521 Such expressions using the ',' operator are dangerous. Make sure the expression 'i < n, i ++' is correct. bllbrbpw.cpp 18

Заметили подвох? Отлично. Если же нет - всё просто. Для правильной работы цикла необходимо разделить операции проверки на условие выхода и инкремент. Здесь же всё перемешалось. Написанная конструкция синтаксически корректна, вот только ни одной итерации цикла выполнено не будет. Дело в том, что условием выхода из цикла будет не выражение 'i<n', как ожидалось, а 'i++'. Так как цикл 'for' выполняется до тех пор, пока условие выхода истинно, здесь не будет выполнено ни одной итерации ввиду того, что при первой же проверке условие будет ложно.

Куда интереснее было бы, если бы начальное значение переменной 'i' было бы равно 1, или была использована операция преинкремента (++i). Тогда цикл выполнялся бы до тех пор, пока значение 'i' не стало бы равно 0 (то есть 'i' должна была "пройти" по всему диапазону данного типа - как положительному, так и отрицательному).

Следующий забавный пример:

int main()
{
  ....
  for (i = 0; i < 255; i++);
  {
    if (eldertext[i] = 'a'){}
  }
  ....
}

Предупреждение анализатора: V529 Odd semicolon ';' after 'for' operator. ryci4ba3.cpp 11

Здесь даже 2 интересных момента:

  • Цикл. Благополучно выполнит все положенные итерации, но бестолку. Причиной всему - поставленная не в том месте точка с запятой. Но даже если бы она была поставлена верно, это не решило бы проблемы.
  • Условие. С присваиванием, вместо сравнения. С пустым телом. Здесь, пожалуй, даже комментировать нечего.

Идём дальше:

int main()
{
  int i, j;
  ....
  for (i = 0; i < 4; i++)
  {
    for (j = 0; j < 5; i++)
    {
      scanf_s("\n%f", A[i][j]);
    }
    scanf_s("\n");
  };
  ....
}

Предупреждение анализатора: V533 It is likely that a wrong variable is being incremented inside the 'for' operator. Consider reviewing 'i'. fdpxaytc.cpp 16

Не будем обращать внимание на точку с запятой, зачем-то поставленную после закрывающей тело цикла скобки, а взглянем на вложенный цикл. Очевидно, что цикл бесконечный. А ошибка - в опечатке. Вместо того, чтобы инкрементировать значение переменной 'j', увеличивается значение 'i'. В итоге условие 'j<5' никогда не будет выполнено. В том же самом файле такой же код встретился несколько раз.

Продолжаем тему бесконечных циклов:

Documento Escritorio::retiraDoc(string user1, string titulo1)
{
  ....
  unsigned int count = 0;
  ....
  while (count >= 0)
  { 
    it->retiraDoc();
    count--;
  }
  ....
}

Предупреждение анализатора: V547 Expression 'count >= 0' is always true. Unsigned type value is always >= 0. 5hyhalvq.cpp 34

Здесь даже неважно, изменяется-ли значение 'count' или нет. Чтобы понять суть ошибки, достаточно взглянуть на тип данной переменной - unsigned int. То есть переменная 'count' не может быть отрицательной, следовательно, при попытке декремента, когда 'count' равна 0, она просто примет максимально допустимое значение. Итог - бесконечный цикл.

Обратный пример - цикл, который не будет выполнен ни разу:

Matrix()
{
  N = 0;
  matrix = new double*[N];
  for (int i = 0; i < N; i++)
  {
    matrix[i] = new double[N];
  }
}

Сообщение анализатора: V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. 6tx445ic.cpp 22

Наблюдаем интересную реализацию конструктора по умолчанию. По-моему, нужно было постараться, чтобы придумать такое.

Указатели и работа с памятью

Следующий минный полигон. Указатели. Достаточно неприятная тема для тех, кто изучает язык "мимоходом". Взглянем на пару примеров:

int main(....)
{
  ....
  int* p = 0;
  *p = 90;
  ....
}

Сообщение анализатора: V522 Dereferencing of the null pointer 'p' might take place. 4ycv0zvb.cpp 10

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

Ещё один пример. Несколько интереснее:

int main() 
{
  Test * t = nullptr;
  t -> hello(); 
  return 0;
}

Сообщение анализатора: V522 Dereferencing of the null pointer 't' might take place. fafhwx5g.cpp 13

Для полноты картины предоставляю объявление класса 'Test'.

class Test 
{
  public:
    static void hello() 
    {
      std::cout << "Hello World!" << std::endl;   
    }
};

К слову, эти строки кода - весь файл. Довольно нетривиальный способ получения стандартного 'Hello world!'.

Встречаются разные ошибки и при работе с памятью. Следует запомнить, что память, выделенную с помощью 'new', следует освобождать с помощью 'delete', 'new[]' - 'delete[]'. Код, где про данное правило забыли:

char *getline()
{
  ....
  char * mtmp = new char[SIZE];
  ....
  delete mtmp;
  ....
}

Сообщение анализатора: V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] mtmp;'. mzxijddc.cpp 40

Как видно из кода, память выделяется с помощью оператора 'new[]', но при этом освобождается при помощи оператора 'delete', что приводит к неопределённому поведению. Стоит отметить, что в других частях кода из того же файла операторы 'new[]' и 'delete[]' сочетаются правильно, что свидетельствует об опечатке в данном случае. Си++ - язык, где всегда нужно держать ухо востро.

А вот пример утечки памяти:

main()
{
  ....
  int *A=new int[n],*B=new int[n],t;
  ....
  delete[]  A,B;
  ....
}

Сообщение анализатора: V680 The "delete A, B" expression only destroys the 'A' object. Then the ',' operator returns a resulting value from the right side of the expression. kdnaggkc.cpp 45

В принципе, всё понятно из сообщения - будет удалён только массив 'A', так как в данном случае используются оператор запятая (','). То есть строка удаления эквивалентна следующему коду:

(delete[] A), B;

Корректное удаление должно выглядеть следующим образом:

delete[] A;
delete[] B;

В результате неправильного применения операторов получили утечку памяти. Насколько большую - зависит от размера массива B.

Пример потенциально опасного использования функции 'realloc()':

Matrix& operator+ (Matrix& a, Matrix& b)
{
  ....
  res.matrix = (double**)realloc(res.matrix,sizeof(double*)*b.m);
  ....
}

Предупреждение анализатора: V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'res.matrix' is lost. Consider assigning realloc() to a temporary pointer. 7d7bnatk.cpp 79

Конечно, тут и помимо 'realloc()' можно придираться... Но речь не о том. Дело в том, что результат функции сохраняется в той же переменной, в которой раньше хранился адрес блока выделенной памяти. Если выделение блока памяти даже с учётом перемещения данных невозможно, функция 'realloc()' вернёт нулевой указатель, который будет записан в переменную, хранившую адрес выделенного блока памяти. Это и есть потенциально опасная ситуация, которая может привести к утечке памяти. Во избежание подобных неприятностей следовало бы хранить результат функции в другой переменной.

Пример проверки 'this' на нулевой указатель:

struct AVLNode 
{
  ....
  int getHeight() 
  {
    return this == 0 ? 0 : height;
  }
  ....
};

Сообщение анализатора: V704 'this == 0' expression should be avoided - this expression is always false on newer compilers, because 'this' pointer can never be NULL. ltxs3ntd.cpp 25

Можно много написать про проверку 'this' на равенство нулевому указателю, но я предлагаю ознакомиться со следующими статьями, где про это достаточно подробно расписано: раз, два.

Прочие ошибки

Ещё один из примеров ошибок:

INT OutputArray(....)
{
  ....
  if (output[i + j] == 0x0D)
  {
    j = j;
  }
  ....
};

Предупреждение анализатора: V570 The 'j' variable is assigned to itself. chrmwjm9.cpp 277

Не будем смотреть на точку с запятой после функции, взглянем на ветвь оператора 'if'. Как видно, значение переменной 'j' присваивается самой себе. Скорее всего, имеет место быть опечатка, и с одной из сторон оператора '=' должна была присутствовать переменная 'i'. Даже если это не ведёт к неправильной работе программы, таких мест следует избегать.

Picture 2

А вот следующую функцию я даже комментировать не хочу. Наверное, её название и отражает суть ('fun' - веселье). Предлагаю взглянуть на этот "забавный" код:

int fun(int p, int q)
{
    int a, b, s;
    s = 0;
    if( p <  0 ) 
    goto a1;
    if( p == 0 ) 
    goto a2;
    if( p >  0 ) 
    goto a3;
  a1: a = -p;
    goto a4;
  a2: a =  0;
    goto a4;
  a3: a = +p;
    goto a4;
  a4: p = a;
    if( q <  0 ) 
    goto b1;
    if( q == 0 ) 
    goto b2;
    if( q >  0 ) 
    goto b3;
  b1: b = -q;
    goto b4;
  b2: b =  0;
    goto b4;
  b3: b = +q;
    goto b4;
  b4: q = b;
  c1: if( a == 0 ) 
    goto c2;
    p = a / 10;
    p = p * 10;
    p = a - p;
    s = s + p;
    a = a / 10;
  c2: a = a;
    if( b == 0 ) 
    goto c3;
    q = b / 10;
    q = q * 10;
    q = b - q;
    s = s - q;
    b = b / 10;
  c3: b = b;
    if( a ) 
    goto c1;
    if( b ) 
    goto c1;
    return 
    s != 0;
}

Вот вам ещё интересный участок:

int main() 
{
  ....
  char valinta = '1'; '2'; '3';
  ....
}

Сообщение анализатора: V606 Ownerless token ''2''. l8xzvux7.cpp 12

Ошибка очевидна. Вопрос в том, как можно было допустить подобную опечатку (хотя, по-моему, на опечатку это похоже слабо), или же как планировался использоваться этот код? Неясно.

Есть ещё один подобный участок, где, однако, очевидна допущенная опечатка:

bool operator < (const Key &lhs, const Key &rhs)
{
  if(....) 
    return true;
  else if(....) 
    return true;
  else if(....) 
    return true;
  else false;
}

Сообщение анализатора: V606 Ownerless token 'false'. 662eljcq.cpp 31

Ошибка, в общем-то, аналогична предыдущей, но здесь явно наблюдается банальная опечатка (пропущен 'return' перед 'false').

Неоднократно попадались подобные участки кода:

int main (void)
{
  int a;
  short b;
  long c;
  printf("Ausgabe der Speicheradressen:");
  printf("\n----------------------------:");
  printf("\n\nVariable 1(d): %d", &a);
  printf("\n\nVariable 1(p): %p", a);
  printf("\nVariable 2(d):  %d", &b);
  printf("\nVariable 2(p):  %p", b);
  printf("\nVariable 3(d):  %d", &c);
  printf("\nVariable 3(p):  %p", c);
  printf("\n\n");
  system("pause");
}

Пример сообщения анализатора: V576 Incorrect format. Consider checking the second actual argument of the 'printf' function. The pointer is expected as an argument. j38r7dqb.cpp 16

Дело в несовпадении строки форматирования и передаваемых в функцию фактических аргументов. Как результат - неопределённое поведение программы. Например, могут быть напечатаны бессмысленные значения.

Заключение

Конечно, это далеко не все ошибки из проанализированных файлов. Но эти, пожалуй, наиболее интересны. Надеюсь, вы узнали из статьи что-то новое и пополнили багаж своих знаний, ведь как говорится - "век живи - век учись".



Найдите ошибки в своем C, C++, C# и Java коде

Предлагаем попробовать проверить код вашего проекта с помощью анализатора кода PVS-Studio. Одна найденная в нём ошибка скажет вам о пользе методологии статического анализа кода больше, чем десяток статей.

goto PVS-Studio;


Найденные ошибки

Проверено проектов
361
Собрано ошибок
13 417

А ты совершаешь ошибки в коде?

Проверь с помощью
PVS-Studio

Статический анализ
кода для C, C++, C#
и Java

goto PVS-Studio;