C/C++: The Hazards of Using a Minimal Amount of Code

This first article in a series devoted to how C/C++ programmers play with fire without knowing it, will discuss how programmers often attempt to explicitly call a constructor.

Programmers are lazy creatures. That's why they tend to solve a task using a minimal amount of code. There's nothing wrong with this approach, in fact, it's good but it's important to not get too involved in the process and to stop at the right time.

For example, programmers are too lazy to create a single initialization function in a class so that it can be called from various constructors later. They think: "Why do I need an extra function? I'd rather call one constructor from the other". Unfortunately, sometimes programmers can't solve even this simple of a task. It is to detect such unsuccessful attempts that I'm implementing a new rule in PVS-Studio. Here, for instance, is a code sample I have found in the eMule project:

class CSlideBarGroup
{
public:
  CSlideBarGroup(CString strName,
    INT iIconIndex, CListBoxST* pListBox);
  CSlideBarGroup(CSlideBarGroup& Group);
  ...
}

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

Let's examine more attentively how the last constructor is implemented. The programmer decided that the following code...

CSlideBarGroup(
  Group.GetName(), Group.GetIconIndex(), Group.GetListBox());

...simply calls the other constructor. Nothing of the kind. A new unnamed object of the CslideBarGroup type is created and destroyed right after.

It appears that the programmer has actually called the other constructor. But he/she has not quite done what he/she intended: the class fields remain uninitialized.

Such errors are only half the trouble. Some people do know how to call the other constructor, and they do it. I wish they didn't know how.

For instance, the above given code could be rewritten in this way:

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

...or in this way:

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

Now one data initialization constructor is really calling the other constructor.

If you see a programmer doing this, deal him/her one flick on his/her forehead for yourself and one more flick on my behalf.

The cited examples contain very dangerous code and you should understand well how they work!

Being written for the purpose of petty optimization (programmers are too lazy to write a separate function), this code might do more harm than good. Let's look more closely as to why such constructs sometimes work but most often don't.

class SomeClass
{
  int x,y;
public:
  SomeClass() { new (this) SomeClass(0,0); }
  SomeClass(int xx, int yy) : x(xx), y(yy) {}
};

This code will work correctly. It is safe and works well, since the class contains primary data types and is not a descendant of other classes. In this case, a double constructor call is harmless.

Let's consider another code where an explicit constructor call causes an error (the sample is taken from the discussion on the StackOverflow website):

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); 
  } 
}

When we call the "new (this) Derived(bar.foo);" constructor, the Base object is already created and fields initialized. The repeated constructor call will cause double initialization. A pointer to the newly allocated memory area will be written into 'ptr'. As a result, we get a memory leak. The result of double initialization of an object of the std::vector type cannot be predicted at all. But one thing is obvious: such code is inadmissible.

Conclusion

An explicit constructor call is needed only in very rare cases. In common programming practice, an explicit constructor call usually appears due to a programmer's wish to reduce the code's size. Don't do that! Create an ordinary initialization function.

This is how the correct code should look:

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());
  }
  ...
};

P.S. Explicit call of one constructor from the other in C++11 (delegation)

The new C++11 standard allows you to perform a call of constructors from other constructors (known as delegation). It allows you to create constructors that use behavior of other constructors without added code. This is an example of the correct code:

class MyClass {
  std::string m_s;
public:
    MyClass(std::string s) : m_s(s) {}
    MyClass() : MyClass("default") {}
};


About the Author

Andrey Karpov

Andrey Karpov is technical manager of the OOO "Program Verification Systems" (Co Ltd) company developing the PVS-Studio tool which is a package of static code analyzers integrating into the Visual Studio development environment.

PVS-Studio is a static analyzer that detects errors in source code of C/C++ applications. There are 3 sets of rules included into PVS-Studio:

  1. Diagnosis of 64-bit errors (Viva64)
  2. Diagnosis of parallel errors (VivaMP)
  3. General-purpose diagnosis

Andrey Karpov is also the author of many articles on the topic of 64-bit and parallel software development. To learn more about the PVS-Studio tool and sources concerning 64-bit and parallel software development, please visit the www.viva64.com site.

Best Articles:

My page on LinkedIn site: http://www.linkedin.com/pub/4/585/6a3

E-mail: karpov@viva64(dot)com

Related Articles

Comments

  • Wholesale Oakley Half X wholesale store

    Posted by zkbwotxyg on 07/09/2013 01:40pm

    Cheap Ray Bans ,Oakley is actually certainly the addition towards the very good reason, the love area of the brand, track and field and magnificence of management of the profession, this can continuous development. Greatly improve the enjoyment and fitness, the lens from the various environmental and functional performance and body protection. Should you be looking to actually escape there, you would like the look of this beautiful heroine, you'll need is really a set of Oakley. Fake Oakley Juliet ,Oakley sunglasses optimization process provides up to 90 9% efficiency, this mode of sunglasses, reduces distortion, plus the haze. Basic and generous, discount Oakley sunglasses, and very quickly all brands took the demand, in addition has become fashionable for many years been a favourite "better product". Whether you could have experience with an oval-shaped or spherical, square, or extensive, you may not fret, Oakley collection, you'll find this can be tailored for an individual. Hitting the ground with the skin, nose pad special high-energy minerals thetemperature reached 32 degrees, you aren't guilty of the special energy and negative ions, to get the antioxidant, and gradually the role of cell activity, and promote head blood circulation, thus effectively relieve eye fatigue . Fak Oakley Sunglasses ,Sun block is usually destined to not merely be aware that you could have already installed a more stylish and much more attractive appearance, which can be actually a powerful way to. The most important and innovative style, code and data variety of posters inside the 2012 sunglasses summer preparation, really the only products which may you're going away the most beneficial partner? Fake ray bans ,Oakley Frogskins sunglasses sunglasses history of by far the most well-known style. It is a new trend now online buy discount Oakley sunglasses. Polarized Oakley sunglasses so popular polarized lenses to get rid of the eye a significant effect, providing customers with additional comprehensive protection. Sac à Dos Longchamp ,A cutting-edge technology, providing about 99%, optimized polarization and Oakley lenses made out of Plutonite to offer the very best protection against harmful ultraviolet and bright glare. Oakley lens science to supply maximum clarity, while protected from harmful ultraviolet rays. Oakley sunglasses, includes a rare power to blend science and art. Oakley sunglasses are the first choice for celebrities, the rich and famous; produce the highest level of care and awareness of detail using quality materials, the sunglasses a feeling of luxury.

    Reply
  • So, no RIAA?

    Posted by Sean Hunt on 04/27/2012 10:04am

    First, I'm an old school game programmer so if I saw code like this I would indeed Gibbs-slap the author of such code, but I don't really know how common this level of (to me) obvious idiocy is perpetrated... Second, RIAA is an important concept; requiring an Init() function call that simply sets up the class is often a warning sign to me. As per Bjarne, the constructors should set up the class for use when intantiated. Also, writing umpteen constructors (or constructors with lots and lots of optional arguments) is usually a sign that the programmer is trying to do things that shouldn't be in the contructor, and would also lead to head-flicking. Thanks, Sean

    Reply
  • Adding destructor call

    Posted by AMK_VT on 03/04/2012 03:20am

    I agree so far with you, that I wouldn't recommend to use double initialization too, 
    but I think the memory leak can be prevented by adding a destructor call.
    
    [Only for illustration - it's IMHO bad code, which shouldn't be used either]
    
    E.g. in the Derived sample
    
      Derived(Bar bar) {
         this->~Derived(); // Prevents memory leak 
         new (this) Derived(bar.foo); 
      } 
    
    If this wouldn't be slow and dangerous code it could 
    be rewritten by using an re-init function:
    
      Derived(Bar bar) 
      {
          Reinit(bar.foo);
      } 
    
      template 
      void Reinit(const T& t)
      {
         this->~Derived();
         new (this) Derived(t); 
      }
    
    [Disclaimer: I wouldn't recommend such slow and dangerous code]
    
    This would be technically comparable to an Init function, which is called by all constructors.
    Which raises the question how can the Derived sample be rewritten by using an Init function, withouth changing the base class and without double initialization?
    
    I think without changing the class structure from derivation to encapsulation (Base class as member variable) only C++11 delegation constructors can prevent double initialization and code rewrite.
    
    I too use initialization functions, mainly for POD types. But as always it's IMHO not a general solution, it's a compromise not using constructors, instead using member assignment in an Init function, what may have an impact on performance.
    
    All in all, Init functions called by constructors can't prevent double initialization, but perhaps prevent code rewrite and unitialized pod members.

    Reply
Leave a Comment
  • Your email address will not be published. All fields are required.

Top White Papers and Webcasts

  • Today's agile organizations pose operations teams with a tremendous challenge: to deploy new releases to production immediately after development and testing is completed. To ensure that applications are deployed successfully, an automatic and transparent process is required. We refer to this process as Zero Touch Deployment™. This white paper reviews two approaches to Zero Touch Deployment--a script-based solution and a release automation platform. The article discusses how each can solve the key …

  • Learn How A Global Entertainment Company Saw a 448% ROI Every business today uses software to manage systems, deliver products, and empower employees to do their jobs. But software inevitably breaks, and when it does, businesses lose money -- in the form of dissatisfied customers, missed SLAs or lost productivity. PagerDuty, an operations performance platform, solves this problem by helping operations engineers and developers more effectively manage and resolve incidents across a company's global operations. …

Most Popular Programming Stories

More for Developers

RSS Feeds