﻿# Breaking down the C\+\+ quiz by Sergei Kushnirenko

The PVS\-Studio team along with Sergei Kushnirenko prepared a quiz based on his publications\. Take the quiz, challenge your focus and coding skills while looking for errors in the C\+\+ code\. This article provides a detailed analysis of all the questions\.

![1091_cpp_quiz_by_Sergei_Kushnirenko/image1.png](https://import.viva64.com/docx/blog/1091_cpp_quiz_by_Sergei_Kushnirenko/image1.png)

If you haven't taken the quiz, but have opened this article, we urge you not to read it just yet\. Don't deny yourself the pleasure of "bug\-spotting", that's the funniest part\.

Take the [quiz](https://pvs-studio.com/en/blog/quest/kushnirenko_quiz/) and come back to us\.

![1091_cpp_quiz_by_Sergei_Kushnirenko/image2.png](https://import.viva64.com/docx/blog/1091_cpp_quiz_by_Sergei_Kushnirenko/image2.png)

Now that you have completed the quiz, you may have questions or not understand some of the tasks and answers\. Make yourself some tea or coffee and we'll take a closer look at the tasks\. But first, let us say a few words of gratitude\.

We created the quiz with the help of Sergei Kushnirenko's articles on Habr\. The PVS\-Studio team came up with the idea of organizing the code fragments from his articles into an entertaining exercise book\. 


> Sergei Kushnirenko is an AI Gameplay Engineer at 4A Games\\\. 
> 
> He develops classic game AIs and enjoys dissecting algorithms as much as dismantling old cars\\\. He took the errors and examples from the projects he worked on\\\. Exceptions are disabled in the examples, and they also have vendor compiler features\\\. As a result, some obvious issues may not be detected, and the programs may not crash but behave strangely\\\. 
> 
> LinkedIn account: \[dalerank\]\(https://www\.linkedin\.com/in/dalerank/\)\\\.

Many thanks to Sergei for taking part in this project\.

## Task N1\. You spin me right round

This code fragment was in the Unity 2014 engine to control an object rotation using gizmo \(a control element that usually has the shape of three circles — it enables you to rotate the object in the scene\)\. The _setDeltaPitch_ function here changes the object angle relative to its vertical axis \(pitch\)\. At angles close to 0 \(depending on the editor settings\), the function simply turned the object upside down, which really annoyed level designers\.

```cpp
void  UObject::setDeltaPitch(const UMatrix &gizmo) {
  ....
  if (_fpzero(amount, eps))
    return
  rotateAccum.setAnglesXYZ(axis);
  ....
}
```

**Here are the answer options:**

1. Actually, everything's correct\. This is just a warm\-up example\.
1. The actual arguments are mixed up in the _\_fpzero_ function\.
1. The _UMatrix_ class uses the _float_ types, accuracy suffers as the result\. Developers should have used _UMatrixD_ based on the _double_ types\.
1. The code design doesn't match the logic of its operation\. \[CORRECT\]

**The answer**

There is no semicolon after the _return_\. So, under certain conditions, it can cause the _SetAnglesXYZ_ function to be called in the controlled object\. As a result, the object would rotate to an arbitrary angle\.

**A note from the PVS\-Studio team**

It seems that errors caused by forgotten semicolons are quite common\. Various articles mention them frequently\. In reality, however, they are very rare\. In more than 10 years of checking open\-source projects, we've found only [3 such errors](https://pvs-studio.com/en/blog/examples/v504/)\. The fact that everyone is aware of such errors and pays attention to them helps\. Analyzers and compilers also seem to detect them well\.

## Task N2\. Hash sums

The compiler had its fun here\. The developers used this function to calculate the hash sum when checking file integrity\. During content creation, a hash of the files is calculated and stored with the files\. Later, when loading the same file, the Unity player would again calculate the hash of the file and compare it to the saved one to ensure that the file hasn't been modified or corrupted\. Encryption was used at the final packaging stage\. The code author believed that the key will not leak outside the function, but something went wrong\. With some of the leaked Unity source code and the bug in the engine, SKIDROW cracked Syberia 3 and several other big Unity games in 2017\. The games used native content encryption tools\. Denuvo was there too, but SKIDROW learned how to bypass it even earlier\.

```cpp
void UContentPackage::createPackage(dsaFile *m, const void *v, int size) {
  unsigned char        secretDsaKey[3096], L;
  const unsigned char *p = v;
  int                  i, j, t;
  ....
  UContent::generateDsaKey(secretDsaKey, sizeof(salt));
  ....
  // there was some code to encrypt and sign the file
  ....
  memset (secretDsaKey, 0, sizeof(secretDsaKey));
}
```

**Here are the answer options:**

1. The _memset_ function is fake\. The above is an intentional vulnerability that overrides the _memset_ function\.
1. The compiler removes the call to _memset_ because it thinks it's not needed here\. \[CORRECT\]
1. The compiler removes the call to _memset_ due to a rare bug\. The next compiler release fixed it\. However, the broken version was used when the bug was discovered\.
1. An error is in the _generateDsaKey_ function\. It overflows the buffer, and some data gets lost somewhere\.

**The answer**

The compiler applied aggressive optimizations and removed the call to the _memset_ function\. No operations with _secretDsaKey_ were executed after _memset_, so the compiler would just optimize the function call out\. All key contents remained on the stack\.

**A note from the PVS\-Studio team**

This case is the exact opposite of the previous error pattern\. The disappearing _memset_ function call seems exotic and rare\. However, it's a very common potential vulnerability\. At the time of writing the article, our collection already counts [330](https://pvs-studio.com/en/blog/examples/v597/) such errors in various open\-source projects, and the number keeps growing\. Although, it seems that a lot has already been written on this topic:

* [CWE\-14](https://cwe.mitre.org/data/definitions/14.html)
* [Zero and forget \-\- caveats of zeroing memory in C](http://web.archive.org/web/20160304012102/http:/www.eliteraspberries.com/blog/2012/10/zero-and-forget--caveats-of-zeroing-memory-in-c.html)
* [Safe Clearing of Private Data](https://pvs-studio.com/en/blog/posts/cpp/0388/)

Private data can also be sent externally, for example, over a network\. How? We discussed this topic in the article: "[Overwriting memory \- why?](https://pvs-studio.com/en/blog/posts/cpp/k0041/)"\.

## Task N3\. Threads

Working in two or more threads with any of the _a/b_ variables can be problematic\. The error was in the CryTek engine when synchronizing car states over the network\. It caused jerky movements and teleportation when players were driving a vehicle in FarCry 1 multiplayer\. The more players were on the map, the more likely the last player would teleport\. With 16 players on the map, the last player would always teleport when using a car\.

```cpp
struct X {
  int a : 2
  int b : 2
} x;

Thread 1:
void foo() { x.a = 1 }

Thread 2:
void boo() { x.b = 1 }
```

**Here are the answer options:**

1. The entries in the _a_ and _b_ fields are non\-atomic\. They can be partially executed, and other operations can interrupt them\. \[CORRECT\]
1. The size of the first field was ignored due to a bug in the compiler\. So, if _a_ was written to a variable, _b_ would also be overwritten\.
1. The developers forgot that a character takes one bit, making the value range of two\-bit variables insufficient: \[\-2\.\.1\]\. They needed to make three\-bit fields\.

**Here's the answer**

The atomicity of the write operation is violated\. The _a_ and _b_ fields are non\-atomic, so they can be partially executed and interrupted by other operations\. This code fragment has shared access to the complex _AB_ variable, which has two two\-bit parts\. However, the compiler can't do such an assignment atomically by grabbing the _ab_ byte and using bitwise operations to set the desired value\. This can lead to data races and undefined behavior in a multithreaded environment\.

```cpp
foo():                                # @foo()
        mov     AL, BYTE PTR [RIP + x] ; broken assignment 1
        and     AL, -4                 ; broken assignment 2
        or      AL, 1                  ; broken assignment 3
        mov     BYTE PTR [RIP + x], AL ; finished
        ret

boo():                                # @boo()
        mov     AL, BYTE PTR [RIP + x]
        and     AL, -13
        or      AL, 4
        mov     BYTE PTR [RIP + x], AL
        ret
```

## Task N4\. Mutex

The following code fragment contains a data race even with a seemingly functional mutex\. The error has been found in the Nintendo Switch 4\.5\.1 firmware and above\. Developers accidentally stumbled across it while trying to speed up the creation of UI texture atlases at the game start\. When they tried to load more than 100 textures, the atlas would break\. However, when there were fewer textures, the atlas would be built without any issues\. The developers still haven't fixed the zombie mutexes on Nintendo Switch\. Moreover, there you couldn't create more than 256 mutexes per application\. So, the system is "fun"\.

```cpp
const size_t maxThreads = 10;
 
void fill_texture_mt(int thread_id, std::mutex *pm) {
  std::lock_guard<std::mutex> lk(*pm);
  
  // Access data protected by the lock.
}
 
void prepare_texture() {
  std::thread threads[maxThreads];
  std::mutex m;
 
  for (size_t i = 0; i < maxThreads; ++i) {
    threads[i] = std::thread(fill_texture_mt, i, &m);
  }
}
```

**Here are the answer options:**

1. The wrong default class was selected for implementing locks\.
1. There is no waiting for threads to be completed\. Therefore, the mutex is removed once the _prepare\_texture_ function finishes, and no thread locking occurs\. \[CORRECT\]
1. The above code is correct\. However, this type of parallelization is inefficient, but that's a different story\.
1. We need to declare the _m_ variable with the _volatile_ specifier\.

**The answer**

The mutex is deleted after exiting the _prepare\_texture_ function\. The OS doesn't react to an inactive mutex because, as a core object, it stays for a while\. The address is valid and contains correct data but doesn't provide actual thread locking\.

Note\. According to the [documentation](https://en.cppreference.com/w/cpp/thread/thread/~thread), destructors for active _std::threads_ are called when exiting the function\. This should cause a crash \(a call to _terminate_\)\. However, according to the code author, this didn't happen in this project\. This is strange, but it doesn't really matter because the code is incorrect anyway\.

## Task N5\. A dangerous function with a variable number of arguments

We can define functions so that they take more arguments when called than when declared\. Such functions are called variadic\. C\+\+ provides two mechanisms for defining a variadic function: a pattern with a variable number of parameters and a C\-style ellipsis as the last parameter declaration\. A rather unpleasant behavior occurred in the popular sound library — FMOD Engine\. The code is given as it was in the source: it seems that the devs wanted to save on templates\. [You can play with the code on the OnlineGDB website](https://onlinegdb.com/v4xxXf2zg)\.

```cpp
int var_add(int first, int second, ...) {
  int r = first + second; 
  va_list va;
  va_start(va, second);
  while (int v = va_arg(va, int)) {
    r += v;
  }
  va_end(va);
  return r;
}
```

**Here are the answer options:**

1. The declaration of the _va_ variable should come right at the beginning of the function, before the _r_ variable\.
1. The last function parameter should be 0\. Otherwise, undefined behavior occurs\.
1. Passing any type other than _int_ results in undefined behavior\.
1. Options 2 and 3 are both correct\. \[CORRECT\]
1. The code fragment is correct\. An error is external \(for example, if somebody has tampered with macros\)\.

**The answer**

This is a C\-style variadic function for adding a series of integers\. The arguments are read until 0 is found\. Calling the function without passing 0 as the last argument \(after the first two arguments\) causes undefined behavior\. Passing any type other than _int_ also results in undefined behavior\.

## Task N6\. unique\_lock

Here we didn't lock anything again, even though it looked like we did\. This code fragment was found in the Metro: Exodus engine\. When working with resources, the game strangely crashed\. The developers found it thanks to bug reports and one clever French guy\.

```cpp
static std::mutex m;
static int shared_resource = 0;
 
void increment_by_42() {
  std::unique_lock<std::mutex>(m);
  shared_resource += 42;
}
```

**Here are the answer options:**

1. Despite the locking, developers need to declare the _shared\_resource_ variable as _volatile_\.
1. An anonymous object is created and destroyed right away\. No locking occurs because it's immediately destroyed\.
1. A new variable named _m_ is created here, it's initialized by default\. So, the mutex isn't captured\. \[CORRECT\]

**The answer**

The code author expects the _std::unique\_lock_ anonymous local variable to lock and unlock the _m_ mutex via RAII\. In fact, the new _m_ variable is created here, it's initialized by default\. So, the mutex isn't captured\. See also: "[Most vexing parse](https://en.wikipedia.org/wiki/Most_vexing_parse)"\.

**A note from the PVS\-Studio team**

The error is quite rare\. PVS\-Studio has the [V1025](https://pvs-studio.com/en/docs/warnings/v1025/) diagnostic rule for detecting it\. However, we have not yet found such an error when checking open\-source projects\. It was interesting to see it, as they say, in its natural habitat :\)\.

## Task N7\. memcmp

The following code fragment was used to calculate region checksums in PathEngine\. In the release build, it had issues on different compilers\. The PlayStation build used a specific flag that masked the issue, while the Xbox build didn't\. Developers found the error when trying to build the library on a PC using Clang\.

```cpp
struct AreaCrc32 {
  unsigned char buffType = 0;
  int crc = 0;
};

AreaCrc32 s1 {};
AreaCrc32 s2 {};

void similarArea(const AreaCrc32 &s1, const AreaCrc32 &s2) {
  if (!std::memcmp(&s1, &s2, sizeof(AreaCrc32))) {
    // ....
  }
}
```

**Here are the answer options:**

1. The answer is simple: it's a typo\. The negation operator \(\!\) is redundant\.
1. The issue relates to the field alignment in the structure\. \[CORRECT\]
1. The pointer size is calculated instead of the structure size\.

**The answer**

The fields in the structure are aligned on a 4\-byte boundary\. Padding bytes, which may be filled with zeros or with garbage, are formed between _buffType_ and _sgs_\.

```cpp
struct S {
  unsigned char buffType = 0;
  char[3] _padding = { compiler don't care about content in release };
  int crc = 0;
};
```

The _memcmp_ function compares memory on a byte\-by\-byte basis, including garbage bytes\. So, we get an unknown result when we compare _s1_ to _s2_\. The PlayStation compiler settings explicitly tell the compiler to fill padding bytes with zeros\. The C\+\+ rules for structures state that we need to use the _operator \=\=_ here\. The _memcmp_ function isn't designed for this task\.

By the way, here's another case of comparing redundancies\. We noticed it once during a code review, and just in time\.

```cpp
class C {
  int i;
  
public:
  virtual void f();
   
  // ...
};
  
void similar(C &c1, C &c2) {
  if (!std::memcmp(&c1, &c2, sizeof(C))) {
    // ...
  }
}
```

In C\+\+, comparing structures and classes using _memcmp_ is a bad idea, especially if they contain a pointer to a virtual function table\.

Let's say there are two different classes inherited from _C_\. Then, if _c1_ and _c2_ are references to instances of these two different classes, they won't be equal, even if all the data is the same \(except for the pointer to the virtual function table\)\.

But wait, there's more\. The presence of an implicit pointer in the class can cause alignment issues \(the presence of padding bytes\)\.

## Task N8\. operator \=


> Just in case, we would like to remind you that this text is based on Sergei Kushnirenko's personal experience and his publications\\\. We would like to avoid questions why the PVS\\\-Studio team asks these things or writes such code\\\. We don't do that :\\\)\\\. Sergei is not a part of the PVS\\\-Studio team\\\. We decided to leave this comment to avoid repeating strange discussions like the one that happened after we posted a translation of a third\\\-party article about "\[Google programmers\]\(https://pvs\-studio\.com/en/blog/posts/0952/\)"\\\.

It should be simple, but things like this often confuse applicants during interviews\.

```cpp
struct S { S(const S *) noexcept; /* ... */ };
  
class T {
  int n;
  S *s1;
  
public:
  T(const T &rhs) : n(rhs.n),
                    s1(rhs.s1 ? new S(rhs.s1) : nullptr) {}
  ~T() { delete s1; }
  
  // ...
  
  T& operator=(const T &rhs) {
    n = rhs.n;
    delete s1;
    s1 = new S(rhs.s1);
    return *this;
  }
};
```

**Here are the answer options:**

1. Writing _return rhs_ would be more correct\.
1. There is no protection against_ _assigning the object to itself in_ operator \=_\.
1. You need to zeroize the _s1_ pointer after _delete s1;_ in _operator \=_\. Otherwise, the double free may occur\. 
1. Options 2 and 3 are both correct\. \[CORRECT\]
1. If it's a 64\-bit application, you can lower the size of the structure by swapping the _n_ and _s1_ fields\. The size will decrease from 16 bytes to 12 bytes\.

**The answer**

There's no check for assigning the object to itself in _operator \=_\. As a result, we delete the data and load some garbage\.

There's another weak point in _operator \=_\. If exceptions are not disabled in the project, _new_ may potentially throw _std::bad\_alloc_\. In this case, _this\-\>s1_ keeps the old and possibly non\-zero value which is then passed to _delete_ in the destructor\. As a result, we get double free\.

**A note from the PVS\-Studio team**

Indeed, when rearranging the fields in a structure or class, sometimes you can reduce their size\. However, it doesn't apply to the class in question\. In a 32\-bit program, the class size is always 8 bytes; in a 64\-bit program, it's always 16 bytes because of the alignment required\. At least, this is true for the most common [data models](https://pvs-studio.com/en/blog/terms/0012/) and architectures\. Learn more about reducing the size of classes and structures: [V802](https://pvs-studio.com/en/docs/warnings/v802/)\.

## Task N9\. A line in DLL

You can catch an unobvious bug in as little as four lines of code\. Especially in Unity Engine, which likes to include common headers in different DLLs and then compare them like this: _super\_secret\(\) \=\= super\_secret\(\)_\. What could possibly go wrong, right? Well, _secret \!\= secret_, and the game doesn't run on Windows Phone\.

```cpp
// header a.h
constexpr inline const char* super_secret(void) {
  constexpr const char *STRING = "string";
  return STRING;
}

// somewhere in a.dll
if (super_secret() == saved_string_ptr)

// somewhere in b.dll
if (super_secret() != saved_string_ptr)
```

**Here are the answer options:**

1. It's in the h\-file, so using this function in different DLLs can be problematic\. Each DLL has its own string instance\. \[CORRECT\]
1. The function returns a pointer to a temporary line, and the pointer can't be used\. Or rather, the use of it leads to undefined behavior\.
1. We need to replace _constexpr_ with _static_\.

**The answer**

The error is caused by using a common a\.h header in different libraries\. Everything works fine as long as the engine is built as a single DLL\. By building two different DLLs, we get two different addresses for _STRING_\. The error here occurs because strings are compared as pointers instead of using the _strcmp_ function\.

**A note from the PVS\-Studio team**

Most of the bugs we've described in this article are related to the game development industry in one way or another\. So, there are probably a lot of game devs reading this article\. The PVS\-Studio team wants to ask you for something\. We have started to implement special diagnostic rules in the analyzer\. They are designed to detect bugs specific to [Unity](https://pvs-studio.com/en/blog/posts/csharp/1071/) and [Unreal Engine](https://pvs-studio.com/en/blog/posts/cpp/1069/) projects\. We appreciate your ideas for new diagnostic rules, so don't hesitate to share them\.

## Task N10\. serialize

The function may have UB with a buffer overflow\. This often happens with the \-O3 optimization for Clang, and doesn't happen with \-O0\. The issue has already been fixed for all optimization modes in Clang 12\.10 and above\. I didn't write the code\. It came up during one of the interviews when we were just having a heart\-to\-heart\.

```cpp
char destBuffer[16];
void serialize(bool x) {
  const char* whichString = x ? "true" : "false";
  const size_t len = strlen(whichString);
  memcpy(destBuffer, whichString, len);
}

int main() {
  bool x;
  serialize(x);
  printf("%s", destBuffer);
  return 0;
}
```

**Here are the answer options:**

1. Since the _x_ variable is not initialized, the program outputs "true" or "false" depending on the optimization keys and moon phase\.
1. Not only "true" or "false", but also an empty string can show up due to undefined behavior\.
1. The code doesn't compile\.
1. Using the uninitialized _x_ variable results in undefined behavior\. There's no point in discussing what the outcome can be\. The program can crash or display garbage — anything is possible\. \[CORRECT\]

**The answer**

The compiler fooled itself here\. So, what's going on? First, remember that _bool_ isn't just 0 or 1\. It's compiler\-dependent\. Clang determines that 0 is false and everything else is true\. A strict compiler optimization converts the _len_ calculation to '_len \= 5 \- x_'\. However, the _x_ variable is not initialized, so we may get '_len \= 5\-7_'\. In this case, calling _memcpy_ causes the program to crash due to a buffer overflow\.

**A note from the PVS\-Studio team**

Programmers are often wrong when they try to predict how undefined behavior will manifest itself\. It can't and shouldn't be done\. Here's a good article on the subject: "[Falsehoods programmers believe about undefined behavior](https://pvs-studio.com/en/blog/posts/cpp/1024/)"\.

## Task N11\. sayHello

What will be displayed as a result of running the program?

```cpp
void sayHello() {
    std::cout << "Hello, World!\n";
}
void sayHello() {
    std::cout << "Goodbye, World!\n";
}
int main() {
    sayHello();
    return 0;
}
```

**Here are the answer options:**

1. Don't mess with my mind\. The code doesn't compile\.
1. The program will display "Hello World"\.
1. The program will display "Hello World" first, followed by "Goodbye, World\!"\. It's a special kind of magic\.
1. There is a trick I already know about\. Or I don't know it yet but can assume\. \[CORRECT\]

**The answer**

The console will display "Goodbye, World\!" when you run the code\. This is because the name of the second _sayHello_ function is written in Unicode, which unfortunately can't always be displayed\. So, this function is [called](https://godbolt.org/z/Ec539oPar) \(Clang, latest\)\.

```cpp
sayHello():                           # @sayHello()
        push    rbp
        mov     rbp, rsp
        mov     rdi, qword ptr [rip + std::cout@GOTPCREL]
        lea     rsi, [rip + .L.str]
        call    std::basic_ostream<char, std::char_traits<char> >&
  std::operator<< <std::char_traits<char> >(std::basic_ostream<char,
  std::char_traits<char> >&, char const*)@PLT
        pop     rbp
        ret
"_Z9sayHellov":                         # @"_Z9say\D0\9Dellov"
        push    rbp
        mov     rbp, rsp
        mov     rdi, qword ptr [rip + std::cout@GOTPCREL]
        lea     rsi, [rip + .L.str.1]
        call    std::basic_ostream<char, std::char_traits<char> >&
  std::operator<< <std::char_traits<char> >(std::basic_ostream<char,
  std::char_traits<char> >&, char const*)@PLT
        pop     rbp
        ret
```

**A note from the PVS\-Studio team**

Anyone interested in learning more about the Unicode nuances is welcome to read: "[Trojan Source attack for introducing invisible vulnerabilities](https://pvs-studio.com/en/blog/posts/cpp/0933/)"\.

## Task N12\. Number modulus

The function has the amazing capability to alter reality depending on how many legs its caller has\.

```cpp
int abs_legs(int my_legs) {
  if (my_legs < 0) {
    return -my_legs;
  }
}
```

**Here are the answer options:**

1. What's there to think about? At the end of the function, there is no _return_ which would be executed for non\-negative numbers\. This results in undefined behavior\. \[CORRECT\]
1. Actually, there is nothing wrong at all, as the code will not compile because of the missing _return_\.
1. Everything is correct\. Since C\+\+20, a function with a single argument is assumed to return that argument unless there is an explicit _return_\. That is, the function will simply return _my\_legs_ by default\. The funny thing is that the code used to be wrong, but was fixed when the developers switched to C\+\+20\.

**The answer**

A value\-returning function should return a value \(hm\!\) from all possible branches, because there will always be a condition that leads to undefined behavior\. 

Here's the correct code:

```cpp
int abs_legs(int my_legs) {
  if (my_legs < 0) {
    return -my_legs;
  }
  return my_legs;
}
```

**A note from the PVS\-Studio team**

Many people think that undefined behavior here boils down to the fact that the function can return a random value\. Let us say once again that this is not the case\. All we can say for sure is that this code contains an error\. It's impossible to predict exactly how it will reveal itself\. For example, the compiler may not generate a function exit statement at all\. Don't you believe? Then you may want to read "[An example of undefined behavior caused by absence of return](https://pvs-studio.com/en/blog/posts/cpp/0917/)"\.

## Task N13\. Where's the money, Lebowski?

The following function doesn't fully expand, leaving its magic behind the compiler veil\.

```cpp
int get_money(int index, const int *pockets) {
  int a = index + pockets[++index];
  // ....
  return a;
}
```

**Here are the answer options:**

1. We're looking at unspecified behavior\. 
1. It's undefined behavior\. \[CORRECT\]
1. Since C\+\+17, this code is correct, and we know what it will compute\.

**The answer**

The compiler may rearrange the evaluation order for _index \+ pockets\[\+\+index\]_, leading to undefined behavior with different optimization settings\. An unordered or undefined sequence of operations will cause a side effect when working with the _index_ variable\.

The same code given in "[Order of evaluation](https://en.cppreference.com/w/cpp/language/eval_order)" is considered to result in undefined behavior:

```cpp
n = ++i + i;      // undefined behavior
```

So, we can rewrite the code like this:

```cpp
int a = index + pockets[index + 1];
++index;
```

Or like this, depending on what we want to get:

```cpp
++index;
int a = index + pockets[index];
```

**A note from the PVS\-Studio team**

Do you know how the following code will behave?

```cpp
printf("%d, %d\n", i++, i++);
```

Are you sure? We invite you to read the following article: "[How deep the rabbit hole goes, or C\+\+ job interviews at PVS\-Studio](https://pvs-studio.com/en/blog/posts/cpp/0722/)"\.

## Task N14\. cin

The code looks correct, but we put it here for a reason, right? :\)

```cpp
void umcp_read_buffer_from_pipe() {
  char bufKernel[12];
  char bufMatrix[12];
  std::cin.width(12);
  std::cin >> bufKernel;
  std::cin >> bufMatrix;
}
```

**Here are the answer options:**

1. Another call to _std::cin\.width\(12\)_ is needed\. 
1. The code works correctly starting with C\+\+20\.
1. Options 1 and 2 are both correct\. \[CORRECT\]
1. The code is correct\. It's just an example to take a little breather :\)
1. The correct way would be to write _std::cin\.width\(12\-1\);_ to take into account the terminal null\.
1. The code results in undefined behavior regardless of the input data\.

**The answer**

In this example, the first read doesn't lead to overflow and fills _bufKernel_ with a truncated string\. But the second read can overflow _bufMatrix_\. To prevent this from happening, let's call _std::cin\.width\(12\);_ before getting the _bufMatrix_\. Or we can safely work via strings\.

```cpp
void umcp_read_buffer_from_pipe() {
  std::string bufKernel, bufMatrix;
  std::cin >> bufKernel >> bufMatrix;
}
```

If you're using C\+\+20, the next function overload is selected and everything goes safely\. Even if you didn't do it on purpose\.\.\.

```cpp
template< class Traits, std::size_t N >
basic_istream<char, Traits>&
  operator>>( basic_istream<char, Traits>& st, signed char (&s)[N] );
```

## Task N15\. display\_string

It seems that in this code fragment, the "we\-don't\-do\-that\-here" programmer simply mixes randomness with absurdity and calls it "display customization"\.

```cpp
std::string str_func();
void display_string(const char *);

void set_display_options() {
  const char *str = str_func().c_str();
  display_string(str);
}
```

**Here are the answer options:**

1. The temporary string object is destroyed, and the pointer to the buffer can no longer be used\. \[CORRECT\]
1. This code doesn't compile since C\+\+11\.
1. If the string is empty, the _str_ pointer is _nullptr_\.

**The answer**

Here _std::string::c\_str_ is called for the temporary _std::string_ object\. The resulting pointer points to freed but possibly still valid memory after destroying the _std::string_ object at the end of the assignment expression\. However, don't count on it\. You will get undefined behavior when dereferencing this pointer\.

We can fix the code like this:

```cpp
std::string str = str_func();
const char *cstr = str.c_str();
display_string(cstr);
```

Or like this:

```cpp
display_string(str_func().c_str())
```

## Task N16\. Bar

Why is it dangerous to go to a bar in the morning?

```cpp
void morning(const std::string &owner) {
  std::fstream bar(owner);
  if (!bar.is_open()) {
    // Handle error
    return;
  }

  bar << "customer";

  std::string str;
  bar >> str;
}
```

**Here are the answer options:**

1. It's fine\. Let's write "customer" and read the same thing\.
1. We'll read the "void" from the stream\. \[CORRECT\]
1. The check whether the stream has been opened is implemented incorrectly\.
1. The code is dangerous if you enable the \-O2 optimization mode\.

**The answer**

Because there is a chance you won't exit the bar\. The data is added to the end of the file and then read from the same file\. However, since the position indicator in the file remains in the same place \(at the end\), an attempt to read the data \(from the end\) of the file leads nowhere\. You have to go to the bar in the evening, then you can exit it without any issues\.

```cpp
void evening(const std::string &owner) {
  std::fstream bar(owner);
  if (!bar.is_open()) {
    // Handle error
    return;
  }

  bar << "customer";

  std::string str;
  bar.seekg(0, std::ios::beg);
  bar >> str;
}
```

## Task N17\. Bard

Will the bard become a drunkard?

```cpp
class Bard {
  int _beer;
  int _meal;

public:
  Bard(int meal) : _meal(meal), _beer(_meal - 1) {}
};
```

**Here are the answer options:**

1. You can't start a class field with an underscore character\. This leads to unexpected consequences when using some compilers\.
1. The _\_meal_ variable, which is not yet initialized, is used to initialize _\_beer_\. \[CORRECT\]
1. Everything is correct\.
1. Everything is correct if there can be no inheritance from this class\.

**The answer**

Class members are initialized in the order of their declaration, not in the order in which they are listed in the constructor initialization list\.

The class member initialization order is broken\. So, the _\_beer_ variable is initialized first, and then _\_meal_\. Trying to read the _\_meal_ value before it's initialized leads to undefined behavior\. Most likely, the bard will get drunk without a snack\.

We can fix it like this:

```cpp
Bard(int meal) : _beer(meal - 1), _meal(meal) {}
```

Another way to fix this is to use the _meal_ variable for initialization everywhere:

```cpp
Bard(int meal) : _meal(meal), _beer(meal - 1) {}
```

We can also swap the class fields:

```cpp
class Bard {
  int _meal;
  int _beer;

public:
  Bard(int meal) : _meal(meal), _beer(_meal - 1) {}
};
```

## Task N18\. Iterators

When would a shot in the leg be particularly effective?

```cpp
std::vector<....> formats;
....
for (auto format = begin(formats), __end = end(formats);
     format != __end; ++format)
{
  if (snd::CodecNamesEq(....)) {
    format->is_stereo = true;
    formats.push_back(stereo_format);
  }
}
```

**Here are the answer options:**

1. Variable names should not contain a double underscore\.
1. After _push\_back_ is called, the iterators become invalid\.
1. We need to write _format\+\+_, not _\+\+format_\. It's important here\.
1. Options 1 and 2 are both correct\. \[CORRECT\]
1. Options 1, 2, and 3 are correct\.

**The answer**

If the _push\_back_ process reallocates inside the _formats_ container, the iterator _format_ points to deleted memory, which causes a lot of issues\.

Moreover, a variable named _\_\_end_ is not a good thing\. Variables containing a double underscore are [reserved](http://eel.is/c++draft/lex#name-3) for a standard library and internal use\.

Note\. It's implied that the used container can invalidate iterators when inserting elements at the end, although this isn't specified in the code\. That's important because [_push\_back_](https://en.cppreference.com/w/cpp/container/list/push_back) in _std::list_ doesn't do that\.

## Thanks everyone

We'd like to thank Sergei Kushnirenko for providing materials for the quiz and this article\. A kind word to Andrey Karpov, Mikhail Gelvih, Phillip Khandeliants, and other PVS\-Studio team members\. Why not actually praise ourselves? :\) Thank you, readers, we hope you had fun\. If you want more stuff like this, follow the [link](https://pvs-studio.com/en/blog/quest/)\.

Drop a comment below to share your funny stories or the weirdest code you've ever come across\.

## Additional links

1. A discussion on some tasks listed here on Reddit: [Hello, World\! Where are your bugs?](https://www.reddit.com/r/cpp/comments/16zval4/hello_world_where_are_your_bugs/)
1. You can use the PVS\-Studio analyzer to find most of the errors described here\. We haven't gone into details of what the analyzer can and can't find and why, because we didn't want to overload the text\. However, if you think that your code might have such unexpected issues, we suggest you [download the analyzer and try it](https://pvs-studio.com/en/pvs-studio/try-free/)\.