Pull to refresh
281.78
PVS-Studio
Static Code Analysis for C, C++, C# and Java

Не зная брода, не лезь в воду. Часть первая

Reading time 3 min
Views 3.8K
Не зная брода, не лезь в воду
Захотелось написать несколько небольших заметок о том, как программисты на Си/Си++ играют с огнем, не подозревая об этом. Первая заметка будет про попытки явно вызвать конструктор.

Программисты — ленивые существа. Поэтому норовят решить задачу минимальным количеством кода. Это похвальное и хорошее стремление. Главное не увлечься процессом и вовремя остановиться.

Например, программистам бывает лень создавать единую функцию инициализации в классе, чтобы затем вызывать её из разных конструкторов. Программист думает: «Зачем мне лишняя функция? Я лучше вызову один конструктор из другого». К сожалению, даже эту простую задачу программисту удается решить не всегда. Для выявлений таких неудачных попыток я как раз сейчас реализовываю в PVS-Studio новое правило. Вот, пример кода, который я обнаружил в проекте eMule:
class CSlideBarGroup
{
public:
  CSlideBarGroup(CString strName,
    INT iIconIndex, CListBoxST* pListBox);
  CSlideBarGroup(CSlideBarGroup& Group);
  ...
}

CSlideBarGroup::CSlideBarGroup(CSlideBarGroup& Group)
{
  CSlideBarGroup(
    Group.GetName(), Group.GetIconIndex(), Group.GetListBox());
}

Рассмотрим внимательнее реализацию последнего конструктора. Программист решил, что код
CSlideBarGroup(
  Group.GetName(), Group.GetIconIndex(), Group.GetListBox());

просто вызывает другой констурктор. Ничего подобного. Здесь создается и тут же уничтожается новый неименованный объект типа CSlideBarGroup.

Получается, что программист действительно вызвал другой конструктор. Вот только сделал он совсем не то, что задумал. Поля класса останутся неинициализированными.

Такие ошибки, это только половина беды. Некоторые знают, как все-таки действительно вызвать другой конструктор. И вызывают. Лучше бы они не знали, как это делается. :)

Например, приведенный код, можно было бы переписать так:
CSlideBarGroup::CSlideBarGroup(CSlideBarGroup& Group)
{
  this->CSlideBarGroup::CSlideBarGroup(
    Group.GetName(), Group.GetIconIndex(), Group.GetListBox());
}

или так:
CSlideBarGroup::CSlideBarGroup(CSlideBarGroup& Group)
{
  new (this) CSlideBarGroup(
    Group.GetName(), Group.GetIconIndex(),
    Group.GetListBox());
}

Теперь действительно один конструктор для инициализации данных вызывает другой конструктор.

Если увидите, программиста, который так делает, отвесьте ему один шелбан в лоб от себя и один от меня лично.

Приведенные примеры являются очень опасным кодом, и нужно хорошо понимать, как они работают!

Из-за мелочной оптимизации (лень писать отдельную функцию), этот код может нанести больше вреда, чем пользы. Рассмотрим подробнее, почему иногда подобные конструкции работают, но чаще нет.
class SomeClass
{
  int x,y;
public:
  SomeClass() { new (this) SomeClass(0,0); }
  SomeClass(int xx, int yy) : x(xx), y(yy) {}
};

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

Рассмотрим другой код, где явный вызов конструктора приводит к ошибке (пример взят из дискуссии на сайте StackOverflow):
class Base 
{ 
public: 
 char *ptr; 
 std::vector vect; 
 Base() { ptr = new char[1000]; } 
 ~Base() { delete [] ptr; } 
}; 
 
class Derived : Base 
{ 
  Derived(Foo foo) { } 
  Derived(Bar bar) { 
     new (this) Derived(bar.foo); 
  } 
}

Когда мы вызываем конструктор «new (this) Derived(bar.foo);», объект Base уже создан и поля инициализированы. Повторный вызов конструктора приведет к двойной инициализации. В 'ptr' запишем указатель на вновь выделенный участок памяти. В результате получаем утечку памяти. К чему приведет двойная инициализация объекта типа std::vector, вообще предсказать сложно. Ясно одно. Такой код недопустим.

Вывод


Явный вызов конструктора требуется только в крайне редких случаях. В обычном программировании, явный вызов конструктора, как правило, появляется из-за желания сокращения размера кода. Не надо этого делать! Создайте обыкновенную функцию инициализации.
Вот как должен выглядеть правильный код:
class CSlideBarGroup
{
  void Init(CString strName, INT iIconIndex,
            CListBoxST* pListBox);
public:
  CSlideBarGroup(CString strName, INT iIconIndex,
                 CListBoxST* pListBox)
  {
    Init(strName, iIconIndex, pListBox);
  }
  CSlideBarGroup(CSlideBarGroup& Group)
  {
    Init(Group.GetName(), Group.GetIconIndex(),
         Group.GetListBox());
  }
  ...
};
Tags:
Hubs:
+90
Comments 91
Comments Comments 91

Articles

Information

Website
pvs-studio.com
Registered
Founded
2008
Employees
31–50 employees