How to avoid mistakes using modern C ++

One of the problems of C ++ is a large number of constructions whose behavior is undefined or just unexpected for a programmer. We often encounter such errors when using a static code analyzer on different projects. But, as you know, it is best to find errors at the compilation stage. Let's see which techniques from modern C ++ allow us to write not only more simple and expressive code, but also make our code more secure and reliable.

What is Modern C ++?

The term Modern C ++ has become very popular since C ++ 11. What does he mean? First of all, Modern C ++ is a set of patterns and idioms that are designed to eliminate the flaws of the good old “C with classes” that many C ++ programmers are used to, especially if they started to program in C. C ++ 11 code looks in many cases more concisely and clearly, that is very important.

What do people usually remember when they talk about Modern C ++? Parallelism, compile-time calculations, RAII, lambdas, ranges (ranges), concepts, modules and other equally important components of the standard library (for example, API for working with the file system). These are very cool innovations, and we are waiting for them in the following standards. At the same time, I would like to pay attention to how new standards allow writing more secure code. When developing a static code analyzer, we encounter a large number of different types of errors, and sometimes the thought arises: "But in modern C ++, this could have been avoided." Therefore, I propose to consider a series of errors found by us with the help of PVS-Studio in various Open Source projects. At the same time and see how to fix them better.
')

Automatic type inference

The keywords auto and decltype have been added to C ++ 11. Of course you know how they work:

std::map<int, int> m; auto it = m.find(42); //C++98: std::map<int, int>::iterator it = m.find(42); 

With the help of auto, you can very conveniently reduce long types without losing readability of the code. However, these keywords are really revealed in combination with templates: c auto or decltype do not need to explicitly specify the type of the return value.

But back to our topic. Here is an example of a 64-bit error :

 string str = .....; unsigned n = str.find("ABC"); if (n != string::npos) 

In a 64-bit application, the value of string :: npos is greater than the maximum value of UINT_MAX , which is contained by an unsigned variable. It would seem that this is the case where auto can save us from this kind of problems: the type of the variable n is not important to us, the main thing is that it accommodates all possible values ​​of string :: find . And indeed, if we rewrite this example with auto , the error will disappear:

 string str = .....; auto n = str.find("ABC"); if (n != string::npos) 

But here is not so simple. Using auto is not a panacea and there are many errors associated with it. For example, you can write this code:

 auto n = 1024 * 1024 * 1024 * 5; char* buf = new char[n]; 

auto will not save from overflow and memory for the buffer will be allocated less than 5GiB.

In a common error with an incorrectly recorded cycle, auto is also not our helper. Consider an example:

 std::vector<int> bigVector; for (unsigned i = 0; i < bigVector.size(); ++i) { ... } 

For large arrays, this cycle becomes infinite. The presence of such errors in the code is not surprising: they manifest themselves in quite rare situations that the tests most likely did not write.

Can this fragment be rewritten via auto ?

 std::vector<int> bigVector; for (auto i = 0; i < bigVector.size(); ++i) { ... } 

No, the error has not gone away. It got even worse.

With simple types of auto behaves very badly. Yes, in the most simple cases (auto x = y) it works, but as soon as additional constructions appear, the behavior may become more unpredictable. And worst of all, the error will be harder to notice, since the types of variables will not be obvious at first glance. Fortunately for static analyzers, calculating the type is not a problem: they do not get tired and do not lose attention. But it is better for mere mortals to indicate simple types explicitly. Fortunately, it is possible to get rid of the narrowing reduction in other ways, but about them a bit later.

Dangerous countof

One of the “dangerous” types in C ++ is an array. Often, when passing it to a function, they forget that it is passed as a pointer, and they try to count the number of elements through the sizeof :

 #define RTL_NUMBER_OF_V1(A) (sizeof(A)/sizeof((A)[0])) #define _ARRAYSIZE(A) RTL_NUMBER_OF_V1(A) int GetAllNeighbors( const CCoreDispInfo *pDisp, int iNeighbors[512] ) { .... if ( nNeighbors < _ARRAYSIZE( iNeighbors ) ) iNeighbors[nNeighbors++] = pCorner->m_Neighbors[i]; .... } 

Note. Code taken from the Source Engine SDK.

PVS-Studio warning : V511 The sizeof () operator returns the sizeof (iNeighbors) expression. Vrad_dll disp_vrad.cpp 60

Such confusion may arise from specifying the size of the array in the argument: this number means nothing to the compiler and is just a hint to the programmer.

The trouble is that such code is compiled and the programmer does not suspect that something is amiss. The obvious solution would be to use metaprogramming:

 template < class T, size_t N > constexpr size_t countof( const T (&array)[N] ) { return N; } countof(iNeighbors); //compile-time error 

In the case when we pass to this function not an array, we get a compilation error. In C ++ 17, you can use std :: size .

In C ++ 11, the std :: extent function was added, but it is not suitable as a countof , since it returns 0 for unsuitable types.

 std::extent<decltype(iNeighbors)>(); //=> 0 

You can make a mistake not only with the countof , but also with the sizeof .

 VisitedLinkMaster::TableBuilder::TableBuilder( VisitedLinkMaster* master, const uint8 salt[LINK_SALT_LENGTH]) : master_(master), success_(true) { fingerprints_.reserve(4096); memcpy(salt_, salt, sizeof(salt)); } 

Note. Code taken from Chromium.

PVS-Studio warnings:

• V511 The sizeof () operator, the sizeof (salt) expression. browser visitedlink_master.cc 968
• V512 A call of the memcpy function will lead to under salt of the buffer salt. browser visitedlink_master.cc 968

As you can see, standard arrays in C ++ have a lot of problems. Therefore, in modern C ++ it is worth using std :: array : its API is similar to std :: vector and other containers and it is more difficult to make a mistake when using it.

 void Foo(std::array<uint8, 16> array) { array.size(); //=> 16 } 

How wrong in simple for

Another source of errors is a simple for loop. It would seem, where there can be mistaken? Is there anything related to the difficult condition of exit or savings on the lines? No, they are mistaken in the simplest cycles.

Let's look at fragments from the projects:

 const int SerialWindow::kBaudrates[] = { 50, 75, 110, .... }; SerialWindow::SerialWindow() : .... { .... for(int i = sizeof(kBaudrates) / sizeof(char*); --i >= 0;) { message->AddInt32("baudrate", kBaudrateConstants[i]); .... } } 

Note. Code taken from Haiku Operation System.

PVS-Studio warning : V706 Suspicious division: sizeof (kBaudrates) / sizeof (char *). Size of each element in the kBaudrates array does not equal to divisor. SerialWindow.cpp 162

We considered such errors in detail in the previous paragraph: again, the size of the array was incorrectly calculated. You can easily fix the situation using std :: size :

 const int SerialWindow::kBaudrates[] = { 50, 75, 110, .... }; SerialWindow::SerialWindow() : .... { .... for(int i = std::size(kBaudrates); --i >= 0;) { message->AddInt32("baudrate", kBaudrateConstants[i]); .... } } 

But there is a better way. In the meantime, look at another piece.

 inline void CXmlReader::CXmlInputStream::UnsafePutCharsBack( const TCHAR* pChars, size_t nNumChars) { if (nNumChars > 0) { for (size_t nCharPos = nNumChars - 1; nCharPos >= 0; --nCharPos) UnsafePutCharBack(pChars[nCharPos]); } } 

Note. Code taken from Shareaza.

PVS-Studio warning : V547 Expression 'nCharPos> = 0' is always true. Unsigned type value is always> = 0. BugTrap xmlreader.h 946

A typical mistake when writing a reverse loop: forgot that the iterator is unsigned and the test returns true always. Perhaps you thought: “How so? Only beginners and students are mistaken. We, professionals, do not have such mistakes. ” Unfortunately, this is not entirely true. Of course, everyone understands that (unsigned> = 0) is true . Where do these errors come from? Often they arise as a result of refactoring. Imagine this situation: the project moves from a 32-bit platform to a 64-bit one. Previously, int / unsigned was used for indexing, and it was decided to replace them with size_t / ptrdiff_t . And in one place they overlooked and used the unsigned type instead of the sign type.

What to do to avoid such a situation in your code? Some advise the use of signed types, as in C # or Qt. Maybe this is a good way, but if we want to work with large amounts of data, the use of size_t cannot be avoided. Is there any safer way to get around an array in C ++? Of course have. Let's start with the simplest: non-member functions. To work with collections, arrays and initializer_list, there are unified functions, the working principle of which you should be familiar with:

 char buf[4] = { 'a', 'b', 'c', 'd' }; for (auto it = rbegin(buf); it != rend(buf); ++it) { std::cout << *it; } 

Fine, now we don’t need to remember the difference between the forward and reverse loop. No need to think about whether we are using a simple array or an array — the loop will work anyway. Using iterators is a good way to get rid of a headache, but even it is not good enough. It is best to use a range for :

 char buf[4] = { 'a', 'b', 'c', 'd' }; for (auto it : buf) { std::cout << it; } 

Of course, the range for has its drawbacks: it does not allow the cycle progress to be managed so flexibly, and if more complex work with indexes is required, this for will not help us. But such situations should be considered separately. Our situation is quite simple: you must go through the elements of the array in the reverse order. However, at this stage difficulties arise. In the standard library there are no helper classes for range-based for . Let's see how it could be implemented:

 template <typename T> struct reversed_wrapper { const T& _v; reversed_wrapper (const T& v) : _v(v) {} auto begin() -> decltype(rbegin(_v)) { return rbegin(_v); } auto end() -> decltype(rend(_v)) { return rend(_v); } }; template <typename T> reversed_wrapper<T> reversed(const T& v) { return reversed_wrapper<T>(v); } 

In C ++ 14, you can simplify the code by removing the decltype . You can see how auto helps writing template functions — reversed_wrapper will work with an array and with std :: vector .

Now you can rewrite the fragment as follows:

 char buf[4] = { 'a', 'b', 'c', 'd' }; for (auto it : reversed(buf)) { std::cout << it; } 

What good is this code? First, it is very easy to read. We immediately see that here the array of elements is reversed. Secondly, it is much more difficult to make a mistake. And thirdly, it works with any type. This is significantly better than what it was.

In boost you can use boost :: adapters :: reverse (arr) .

But back to the original example. There the array is passed in a pointer-size pair. Obviously, our solution with reversed for it will not work. What to do? Use classes like span / array_view . In C ++ 17 there is a string_view , I suggest them and use:

 void Foo(std::string_view s); std::string str = "abc"; Foo(std::string_view("abc", 3)); Foo("abc"); Foo(str); 

string_view does not own a string, in fact it is a wrapper over const char * and length. Therefore, in the example code, the string is passed by value, not by reference. A key feature of string_view is compatibility with different ways of representing strings: const char * , std :: string, and non-null-terminated const char * .

As a result, the function takes the following form:

 inline void CXmlReader::CXmlInputStream::UnsafePutCharsBack( std::wstring_view chars) { for (wchar_t ch : reversed(chars)) UnsafePutCharBack(ch); } 

When passing to a function, it is important not to forget that the constructor string_view (const char *) is implicit, so you can write like this:

 Foo(pChars); 

Not so:

 Foo(wstring_view(pChars, nNumChars)); 

The string pointed to by string_view does not have to be null-terminated, as hinted at by the name of the method string_view :: data , and this should be kept in mind when using it. By passing its value to a function from cstdlib that expects a C string, you can get an undefined behavior. And you can easily skip this if in most cases you are testing, std :: string or null-terminated strings are used.

Enum

Let's take a look at C ++ and remember the good old C. How is security there? After all, there are no problems with implicit calls of constructors and transformation operators, and there are no problems with different kinds of strings. In practice, errors are often found in the simplest structures: the most complex are already carefully reviewed and debugged, as they cause suspicion. At the same time, simple designs often forget to check. Here is an example of a dangerous construction that came to us from C:

 enum iscsi_param { .... ISCSI_PARAM_CONN_PORT, ISCSI_PARAM_CONN_ADDRESS, .... }; enum iscsi_host_param { .... ISCSI_HOST_PARAM_IPADDRESS, .... }; int iscsi_conn_get_addr_param(...., enum iscsi_param param, ....) { .... switch (param) { case ISCSI_PARAM_CONN_ADDRESS: case ISCSI_HOST_PARAM_IPADDRESS: .... } return len; } 

Linux kernel example. PVS-Studio warning : V556 The values ​​of different enum types are compared: switch (ENUM_TYPE_A) {case ENUM_TYPE_B: ...}. libiscsi.c 3501

Note the values ​​in the switch-case : one of the named constants is taken from another enumeration. In the original, of course, the code and the possible values ​​are much larger and the error is not so visual. The reason for this is the non-strict typing of enum - they can be implicitly cast to int , and this gives a great scope for various errors.

In C ++ 11, it is possible and necessary to use the enum class : such a trick will not work with them, and the error will manifest itself during compilation. As a result, the code below does not compile, which is what we need:

 enum class ISCSI_PARAM { .... CONN_PORT, CONN_ADDRESS, .... }; enum class ISCSI_HOST { .... PARAM_IPADDRESS, .... }; int iscsi_conn_get_addr_param(...., ISCSI_PARAM param, ....) { .... switch (param) { case ISCSI_PARAM::CONN_ADDRESS: case ISCSI_HOST::PARAM_IPADDRESS: .... } return len; } 

The following fragment is not entirely related to enum , but has similar symptoms:

 void adns__querysend_tcp(....) { ... if (!(errno == EAGAIN || EWOULDBLOCK || errno == EINTR || errno == ENOSPC || errno == ENOBUFS || errno == ENOMEM)) { ... } 

Note. Code taken from ReactOS.

Yes, errno values ​​are declared by macros, which in itself is a bad practice in C ++ (and in C too), but even if you used enum , it would not have been any easier. The lost comparison does not appear in the case of enum (and even more so the macro). But the enum class would not allow this, since an implicit cast to bool will not occur.

Initialization in the constructor

But back to the original C ++ problems. One of them manifests itself when it is necessary to initialize an object in a similar way in several constructors. A simple situation: there is a class, there are two constructors, one of them calls the other. Everything looks logical: the common code is put into a separate method - no one likes to duplicate the code. What's the catch?

 Guess::Guess() { language_str = DEFAULT_LANGUAGE; country_str = DEFAULT_COUNTRY; encoding_str = DEFAULT_ENCODING; } Guess::Guess(const char * guess_str) { Guess(); .... } 

Note. The code is from LibreOffice.

PVS-Studio warning: V603 The object was not used. If you wish to call constructor, 'this-> Guess :: Guess (....)' should be used. guess.cxx 56

A trick in the syntax of the constructor call. Often they forget about it and create another instance of the class, which will be immediately destroyed. That is, the initial instance does not initialize. Naturally there are 1000 and 1 ways to fix this. For example, you can explicitly call a constructor through this or put everything into a separate function:

 Guess::Guess(const char * guess_str) { this->Guess(); .... } Guess::Guess(const char * guess_str) { Init(); .... } 

By the way, an explicit re-call of the constructor, for example, through this is a dangerous game and you need to understand well what is happening. The variant with the Init () function is much better and clearer. For those who want to deal in more detail with the dirty tricks, I suggest to get acquainted with the 19th chapter "How to call one constructor from another correctly" from this book .

But it is best to use a delegation of designers. So we can explicitly call one constructor from another:

 Guess::Guess(const char * guess_str) : Guess() { .... } 

Such constructors have several limitations. First: the delegated constructor takes full responsibility for initializing the object. That is, with it, initializing another class field in the initialization list will not work:

 Guess::Guess(const char * guess_str) : Guess(), m_member(42) { .... } 

And naturally, it is necessary to ensure that the delegation does not form a cycle, since it will not work out of it. Unfortunately, this code is compiled:

 Guess::Guess(const char * guess_str) : Guess(std::string(guess_str)) { .... } Guess::Guess(std::string guess_str) : Guess(guess_str.c_str()) { .... } 

About virtual functions

Virtual functions pose a potential problem: the fact is that it is very easy in the inherited class to make a mistake in the signature and, as a result, not to redefine the function, but to declare a new one. Consider this situation by example:

 class Base { virtual void Foo(int x); } class Derived : public class Base { void Foo(int x, int a = 1); } 

The Derived :: Foo method cannot be called by the pointer / reference to Base . But this example is simple and one can say that no one is wrong. And usually make mistakes like this:

Note. Code taken from MongoDB.

 class DBClientBase : .... { public: virtual auto_ptr<DBClientCursor> query( const string &ns, Query query, int nToReturn = 0 int nToSkip = 0, const BSONObj *fieldsToReturn = 0, int queryOptions = 0, int batchSize = 0 ); }; class DBDirectClient : public DBClientBase { public: virtual auto_ptr<DBClientCursor> query( const string &ns, Query query, int nToReturn = 0, int nToSkip = 0, const BSONObj *fieldsToReturn = 0, int queryOptions = 0); }; 

PVS-Studio warning : V762 Consider inspecting virtual function arguments. See seventh argument of the function 'query' in the derived class 'DBDirectClient' and base class 'DBClientBase'. dbdirectclient.cpp 61

There are many arguments and there is no last argument in the function of the class of the heir These are already two different unrelated functions. Very often, this error manifests itself with arguments that have a default value.

In the next fragment, the situation is more cunning. Such code will work if it is compiled as 32-bit, but will not work in the 64-bit version. Initially, the parameter in the base class was a DWORD type, but then it was fixed to DWORD_PTR . And in the inherited classes did not change. Long live the sleepless night, debugging and coffee!

 class CWnd : public CCmdTarget { .... virtual void WinHelp(DWORD_PTR dwData, UINT nCmd = HELP_CONTEXT); .... }; class CFrameWnd : public CWnd { .... }; class CFrameWndEx : public CFrameWnd { .... virtual void WinHelp(DWORD dwData, UINT nCmd = HELP_CONTEXT); .... }; 

It is possible to make a mistake in the signature in more extravagant ways. You can forget to const a function or argument. You can forget that the function in the base class is not virtual. You can confuse signed / unsigned type.

In C ++ 11, several keywords have been added that can govern the redefinition of virtual functions. Override will help us. Such code is simply not compiled.

 class DBDirectClient : public DBClientBase { public: virtual auto_ptr<DBClientCursor> query( const string &ns, Query query, int nToReturn = 0, int nToSkip = 0, const BSONObj *fieldsToReturn = 0, int queryOptions = 0) override; }; 

Null vs nullptr

Using NULL to indicate a null pointer leads to a number of unexpected situations. The fact is that NULL is a normal macro, which is expanded to 0, having the type int . From here it is easy to understand why in this example the second function is selected:

 void Foo(int x, int y, const char *name); void Foo(int x, int y, int ResourceID); Foo(1, 2, NULL); 

But even though this is understandable, it is certainly not logical. Therefore, there is a need for nullptr , which has its own type of nullptr_t . Therefore, to use NULL (and even more so 0 ) in modern C ++ is absolutely impossible.

Another example: NULL can be used to compare with other integer types. Imagine that there is a certain WinAPI function that returns a HRESULT . This type has nothing to do with the pointer, so comparing it with NULL does not make sense. And nullptr this underscores the compilation error, while NULL works:

 if (WinApiFoo(a, b) != NULL) //  if (WinApiFoo(a, b) != nullptr) // ,  //  

va_arg

There are situations when an indefinite number of arguments must be passed to a function. A typical example is the formatted input / output function. Yes, it can be designed in such a way that a variable number of arguments is not needed, but I see no reason to abandon this syntax, since it is much more convenient and intuitive. What do old C ++ standards offer us? They suggest using va_list . What problems may arise? It is very easy to pass an argument of the wrong type to such a function. Or do not pass the argument. Let's look more at the fragments.

 typedef std::wstring string16; const base::string16& relaunch_flags() const; int RelaunchChrome(const DelegateExecuteOperation& operation) { AtlTrace("Relaunching [%ls] with flags [%s]\n", operation.mutex().c_str(), operation.relaunch_flags()); .... } 

Note. Code taken from Chromium.

PVS-Studio warning : V510 The "AtlTrace" function is not expected. delegate_execute.cc 96

They wanted to print the string std :: wstring , but forgot to call the c_str () method. That is, the wstring type will be interpreted as const wchar_t * in the function. Naturally, nothing good will come of it.

 cairo_status_t _cairo_win32_print_gdi_error (const char *context) { .... fwprintf (stderr, L"%s: %S", context, (wchar_t *)lpMsgBuf); .... } 

Note. Code taken from Cairo.

PVS-Studio warning : V576 Incorrect format. Consider checking the fwprintf function. Wchar_t type symbols is expected. cairo-win32-surface.c 130

In this snippet, the format specifiers for strings are confused. The fact is that in Visual C ++ for wprintf % s expects wchar_t * , and% S - char * . It is noteworthy that these errors are in the lines intended for displaying errors or debugging information - for sure these are rare situations, so they missed them.

 static void GetNameForFile( const char* baseFileName, const uint32 fileIdx, char outputName[512] ) { assert(baseFileName != NULL); sprintf( outputName, "%s_%d", baseFileName, fileIdx ); } 

Note. Code taken from CryEngine 3 SDK.

PVS-Studio warning: V576 Incorrect format. Consider checking the fourth argument of the 'sprintf' function. The SIGNED integer type argument is expected. igame.h 66

Equally easy to confuse and integer types. Especially when their size depends on the platform. Here, however, everything is more banal: mixed sign and unsigned types. Large numbers will be printed as negative.

 ReadAndDumpLargeSttb(cb,err) int cb; int err; { .... printf("\n - %d strings were read, " "%d were expected (decimal numbers) -\n"); .... } 

Note.Code taken from Word for Windows 1.1a.

PVS-Studio warning: V576 Incorrect format. A different number of actual arguments is expected while calling 'printf' function. Expected: 3. Present: 1. dini.c 498

Example found in one of the archaeological research . The string implies the presence of three arguments, but they are not. Maybe they wanted to print out the data on the stack, but it’s still not worth making such assumptions about what lies there. Definitely need to pass arguments explicitly.

 BOOL CALLBACK EnumPickIconResourceProc(<br> HMODULE hModule, LPCWSTR lpszType, <br> LPWSTR lpszName, LONG_PTR lParam)<br>{<br> ....<br> swprintf(szName, L"%u", lpszName);<br> ....<br>} 

Note.Code taken from ReactOS.

PVS-Studio warning: V576 Incorrect format. Consider checking the swprintf function. To print the value of the% p 'should be used. dialogs.cpp 66

Example of a 64-bit error. The size of the pointer depends on the architecture and using% u for it is a bad idea. What to use instead? The analyzer itself tells us the correct answer -% p. Well, if the pointer is simply printed for debugging. It will be much more interesting if it is then attempted to be read and used from the buffer.

What are bad functions with a variable number of arguments? Almost everyone! They can not check the type of the argument or the number of arguments. Step left, step right - undefined behavior.

, . -, variadic templates . , . printf , :

 void printf(const char* s) { std::cout << s; } template<typename T, typename... Args> void printf(const char* s, T value, Args... args) { while (s && *s) { if (*s=='%' && *++s!='%') { std::cout << value; return printf(++s, args...); } std::cout << *s++; } } 

: . variadic templates , .

, , , — std::initializer_list . . , :

 void Foo(std::initializer_list<int> a); Foo({1, 2, 3, 4, 5}); 

, begin , end for .

Narrowing

(narrowing) . , 64- . , . : . , .

 char* ptr = ...; int n = (int)ptr; .... ptr = (char*) n; 

64- . : . :

 virtual int GetMappingWidth( ) = 0; virtual int GetMappingHeight( ) = 0; void CDetailObjectSystem::LevelInitPreEntity() { .... float flRatio = pMat->GetMappingWidth() / pMat->GetMappingHeight(); .... } 

Note. Source Engine SDK.

PVS-Studio: V636 The expression was implicitly cast from 'int' type to 'float' type. Consider using a fractional part. An example: double A = (double) (X) / Y ;. Client (HL2) detailobjectsystem.cpp 1480

, — . C++11 : . .

 float flRatio { pMat->GetMappingWidth() / pMat->GetMappingHeight() }; 

No news is good news

. — . C++ . — , . :

 void AccessibleContainsAccessible(....) { auto_ptr<VARIANT> child_array( new VARIANT[child_count]); ... } 

Note. Chromium.

PVS-Studio: V554 Incorrect use of auto_ptr. The memory is allocated with 'new []' will be cleaned using 'delete'. interactive_ui_tests accessibility_win_browsertest.cc 171

, : , std::auto_ptr . , deprecated C++11, C++17 . - , auto_ptr : , delete , delete[] . auto_ptr unique_ptr , , deleter , delete , . , ?

 void text_editor::_m_draw_string(....) const { .... std::unique_ptr<unsigned> pxbuf_ptr( new unsigned[len]); .... } 

Note. nana.

PVS-Studio: V554 Incorrect use of unique_ptr. The memory is allocated with 'new []' will be cleaned using 'delete'.text_editor.cpp 3137

It turns out that you can make exactly the same mistake. Yes, it is enough to write unique_ptr <unsigned []> and it will disappear, nevertheless, the code also compiles in this form. That is, it is also possible to make a mistake in this way, but as practice shows, if somewhere it is possible to make a mistake, they will necessarily make a mistake. The code snippet only confirms this. So, using unique_ptr with arrays, be extremely careful: it is easier to shoot yourself in the foot than it seems. Maybe then it is better to use std :: vector according to the principles of Modern C ++?

Consider another type of accident.

 template<class TOpenGLStage> static FString GetShaderStageSource(TOpenGLStage* Shader) { .... ANSICHAR* Code = new ANSICHAR[Len + 1]; glGetShaderSource(Shaders[i], Len + 1, &Len, Code); Source += Code; delete Code; .... } 

Note. Unreal Engine 4.

PVS-Studio: 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 [] Code;'. openglshaders.cpp 1790

: , new[] , free .

 bool CxImage::LayerCreate(int32_t position) { .... CxImage** ptmp = new CxImage*[info.nNumLayers + 1]; .... free(ptmp); .... } 

Note.Code taken from CxImage.

PVS-Studio warning: V611 Consider inspecting operation logics behind the 'ptmp' variable. ximalyr.cpp 50

And in this fragment malloc / free and new / delete were confused . This can happen during refactoring: there were functions from C everywhere, we decided to change, we got UB.

 int settings_proc_language_packs(....) { .... if(mem_files) { mem_files = 0; sys_mem_free(mem_files); } .... } 

Note.Code taken from Fennec Media.

PVS-Studio warning : V575 The null pointer is passed into the 'free' function. Inspect the first argument. settings interface.c 3096

. , . . : . free ( ).

 ETOOLS_API int __stdcall ogg_enc(....) { format = open_audio_file(in, &enc_opts); if (!format) { fclose(in); return 0; }; out = fopen(out_fn, "wb"); if (out == NULL) { fclose(out); return 0; } } 

But the problem concerns not only memory management, but also resource management. You can, for example, forget to close the file, as in the fragment above. And the key word in both cases is RAII. The same concept stands behind smart pointers. In combination with move-semantics, RAII eliminates many memory leak errors. And the code written in this style allows you to more clearly determine the ownership of the resource.

As a small example, I’ll give a wrapper over FILE that uses the capabilities of unique_ptr :

 auto deleter = [](FILE* f) {fclose(f);}; std::unique_ptr<FILE, decltype(deleter)> p(fopen("1.txt", "w"), deleter); 

( ). , C++17 API — std::filesystem . fread/fwrite i/o-, unique_ptr File , , .

?

C++ , . compile-time . .

. ++ , , . - : , - , - .

. PVS-Studio: Linux : , . Windows- Visual Studio, trial-.


, : Pavel Belikov. How to avoid bugs using modern C++ .