Grounded pointers

Not so long ago, one of the employees left our team and joined a company engaged in the development of software related to embedded systems. There is nothing special in this, always and everywhere, someone leaves, and someone comes. It all depends on the number of buns, convenience and preferences. Interesting is another. The person sincerely worries about the state of the code in the new place of work, which as a result resulted in this joint article. It's hard, “easy to program,” when you know what static code analysis is.

Nature reserves

It seems to me that an interesting situation has developed in the world. What happens if the programming department is just a small auxiliary element that does not have a direct relationship to the main activity of the organization? There is a reserve. The field of activity of an organization can be as important and responsible as desired (medicine, military equipment). Anyway, a swamp is formed, where new ideas are fastened and technologies of 10 years old are used.

')

Let me give you a couple of strokes from the correspondence with one person working in the programming department at nuclear power plants:



And he says to me: Why do we need git? Look, I have everything written in the notebook.



...



Do you even have some version control?



2 people use git. The rest of the office is numbered zip at best. Although about zips, I’m only sure about 1 person.



Please do not be afraid. Software developed at nuclear power plants is different, plus no one doesn’t cancel hardware protection. In this department are engaged in the collection and processing of statistical data. But still, the waterlogging trend is clearly visible. I do not know why this is happening, but it is. Moreover, the larger the company, the more pronounced this effect.



I want to emphasize that stagnation in large organizations is an international phenomenon. For foreigners, things are exactly the same. I searched for a long time, but could not find one very suitable article. I don't remember the name either. If someone prompts, add a link. In it, the programmer tells the story of how he worked in the same military department. The ministry was naturally terribly secret and terribly bureaucratic. So secret and bureaucratic, that for several months they could not agree on what rights he should be allocated to work with a computer. As a result, he wrote a program in Notepad (without compiling). And then he was fired for inefficiency.

Forester

Let's return to our former employee. Having come to a new job, he experienced a little culture shock. It’s hard to see how even compiler warnings are ignored after messing around with static analysis tools. In general, it is like an isolated world, where they program by their canons and often use their own terms. Most of his stories I liked the phrase "grounded pointers." Traced proximity to the hardware.



We are proud that we have raised in our team a qualified specialist who cares about the quality and reliability of the code. He did not accept the situation in silence, but tries to correct it.



For starters, he did the following. He read the compiler warnings. Then he checked the project using Cppcheck . In addition to making corrections, he thought about preventing typical errors.



One of the first steps was to prepare them a document aimed at improving the quality of the generated code. Another step could be the introduction of a static code analyzer into the development process. About PVS-Studio is out of the question. First is Linux. Secondly, selling software to similar organizations is not easy. Bye, the choice fell on Cppcheck. This is a very good tool for the first acquaintance of people with the methodology of static analysis.



I propose to get acquainted with the document “How not to write programs” prepared by him. Many points may seem written in the style of a captain of evidence. However, these are real problems that he is trying to prevent.

How not to write programs

Item N1

Ignoring compiler warnings. With their large list, you can easily miss the real program errors that appeared in the newly written code. Therefore, all warnings should be eliminated.

Item N2

In the condition of the 'if' operator, not the value check, but the assignment:

if (numb_numbc[i] = -1) { } 

In this case, the code is compiled, but the compiler issues a warning . Such record will be correct:

 if (numb_numbc[i] == -1) { } 

Item N3

Writing the form “using namespace std;” in header files may cause this scope to be used in all files that include this header file. This may result in the wrong functions being selected or a naming conflict.

Item N4

Comparing signed and unsigned variables:

 unsigned int BufPos; std::vector<int> ba; .... if (BufPos * 2 < ba.size() - 1) { } 

Remember that when mixing signed and unsigned variables:

• overflow may occur;
• a true or false condition can always arise and, as a consequence, an eternal cycle;
• a value greater than INT_MAX may be placed in the signed variable (and it will have a negative value);
• an int variable when adding / subtracting / ... with an unsigned type variable also becomes unsigned (negative values ​​turn into large positive numbers);
• other surprises and joys

The above example incorrectly handles the situation when the array 'ba' is empty. The expression "ba.size () - 1" has unsigned type size_t. If there are no elements in the array, the result of the expression is 0xFFFFFFFFu.

Item N5

Neglecting the use of constancy can make it impossible to notice difficult to fix errors. For example:

 void foo(std::string &str) { if (str = "1234") { } } 

In this example, the operator '=' is confused with the operator '=='. If the variable 'str' were declared as constant, then such code would not even compile.

Item N6

It is not strings that are compared, but pointers to strings:

 har TypeValue [4]; ... if (TypeValue == "S") {} 

Even if the string “S” is in the TypeValue variable, such a comparison will always return 'false'. It would be proper to use functions for comparing strcmp or strncmp strings.

Item N7

Overflow buffer:

 memset(prot.ID, 0, sizeof(prot.ID) + 1); 

Such a code can lead to the fact that several bytes of memory located after 'prot.ID' will also be filled with zeros.



Do not confuse sizeof () and strlen (). The sizeof () operator returns the full size of the object in bytes. The strlen () function returns the length of a string in characters (not including terminal zero).

Item N8

Insufficient buffer filling:

 struct myStruct { float x, y, h; }; myStruct *ptr; .... memset(ptr, 0, sizeof(ptr)); 

In this case, not the whole '* ptr' structure will be filled with zeros, but only N bytes (N is the pointer size in this platform). Such code will be correct:

 myStruct *ptr; .... memset(ptr, 0, sizeof(*ptr)); 

Item N9

Invalid expression:

 if (0 < L < 2 * M_PI) { } 

From the point of view of the compiler, there is no error here, but it does not make sense; when executed, it will always get the value 'true' or 'false' depending on the comparison operators and boundary conditions. The compiler issues a warning for such an entry. It will be correct to write this code like this:

  if (0 < L && L < 2 * M_PI) { } 

Item N10

 unsigned int K; .... if (K < 0) { } ... if (K == -1) { } 

Unsigned variables cannot be less than zero.

Item N11

Comparing a variable with a value that it cannot achieve under any circumstances. Example:

 short s; ... If (s==0xaaaa) { } 

The compiler warns about such cases.

Item N12

We allocate memory through 'new' or 'malloc' and forget to free it through 'delete' / 'free', respectively. For example, there may be such code:

 void foo() { std::vector<int> *v1 = new std::vector<int>; std::vector<int> v2; v2->push_back(*v1); ... } 

Most likely, a pointer to 'std :: vector <int>' was previously stored in 'v2'. Now, due to changing part of the code, this is not necessary and just the values ​​of the 'int' type are saved. At the same time, we do not release the memory allocated for 'v1', because earlier it was not necessary. In order to make the code correct, add the expression 'delete v1' to the end of the function. Or use smart pointers.



Better yet, bring refactoring to the end and make 'v1' a local object, since it no longer needs to be transferred anywhere:

 void foo() { std::vector<int> v1; std::vector<int> v2; v2->push_back(v1[0]); ... } 

Item N13

Memory allocation through 'new []', and release through 'delete'. Or vice versa the selection is 'new', and the release through 'delete []'. Why this is bad, you can read here: " delete, new [] in C ++ and urban legends about their combination ."

Item N14

Using uninitialized variables:

 int sum; ... for (int i = 0; i < 10; i++) { sum++; } 

In C / C ++, the default variable is not initialized to zero. Sometimes it may seem that this code works. This is not true. This is just luck.

Item N15

Return from a function of a link or pointer to local objects:

 char* CreateName() { char FileName[100]; ... return FileName; } 

After exiting the function 'FileName' will point to the already freed memory, since all local objects are created on the stack, and further correct work with it will be impossible.

Item N16

Do not check the return value from functions that may return an error code or '-1' in case of an error. In case of incorrect operation, it may happen that the function returns an error code, but we will not react to it at all and continue working, and then the program will exit incorrectly in a completely unpredictable place. Such moments can be debugged for a long time afterwards.

Item N17

Neglecting the use of special static and dynamic analysis tools, as well as the writing and use of Unit tests.

Item N18

We are greedy to put brackets in mathematical expressions. The result is:

 D = ns_vsk.bit.D_PN_ml + (int)(ns_vsk.bit.D_PN_st) << 16; 

In this case, the first will be the addition, and only then shift to the left. See " Priority operations in the C / C ++ language ". Based on the logic of the program, the sequence of operations should be the opposite - first a shift, and why addition. A similar error occurs in this code:

 #define A 1 #define B 2 #define TYPE A | B if (type & TYPE) { } 

Here the error lies in the fact that the macro TYPE was forgotten surrounded by parentheses. Therefore, the 'type & A' expression will be executed first, and only then why (type & A) | B '. The result - the condition is always true.

Item N19

Array overrun:

 int mas[3]; mas[0] = 1; mas[1] = 2; mas[2] = 3; mas[3] = 4; 

The expression 'mas [3] = 4;' refers to a non-existent element of the array, since when declaring the array 'int mas [N]' the indexing of its elements is possible in the interval [0 ... N-1].

Item N20

The priorities of logical operations '&&' and '||' are confused. The operator '&&' has a higher priority. Bad code example:

 if (A || B && C) { } 

Expectations may not match the required logic of the program. It is often assumed that logical expressions are executed from left to right. The compiler also warns about such suspicious places .

Item N21

The result of the assignment will have no effect outside of the function:

 void foo(int *a, int b) { If (b == 10) { *a = 10; } else { a = new int; } } 

Pointer 'a' can not be assigned a different value of the address, for this should be recorded function declaration in this form:

 void foo(int *&a, int b) {....} 

or:

 void foo(int **a, int b) {....} 

List of recommended literature:

1. ROPE OF SUFFICIENT LENGTH TO SHOOT YOURSELF. C and C ++ Programming Rules. Alain I. Golub;
2. C ++ programming standards. 101 rule and recommendation. Coat of Arms Sutter, Andrei Alexandrescu;
3. Perfect code. S. McConnell;
4. Slippery places C ++. Stefan C. Dewhurst;
5. Effective use of C ++. 50 recommendations for improving your programs and projects. Scot myers

Conclusion

I have no particular conclusions. I only know that somewhere in one particular place, now the situation with software development will be better. It's nice.



A little spoils the mood that many people have not even heard about static analysis. Moreover, often these people are engaged in serious and responsible things. The programming sphere is developing very actively. As a result, those who constantly “work” do not manage to follow trends and tools. They start working much less efficiently than freelancing programmers, start-ups, small companies.



So it turns out a strange picture. The young freelancer can do the job more qualitatively (thanks to the knowledge: TDD, continuous integration, static analysis, version control system, ...) than a programmer who has worked at Russian Railways / AES for 10 years / (substitute something major). Thank God, this is not always the case. But still there is something like that.



Why does it upset me? There would sell PVS-Studio. And there they are not even aware of the existence and usefulness of such tools. :)