Intel developers govodayat too

Recently, talking about checking the next project, I kept repeating that this is a very high-quality code and almost no errors were found in it. An example is the analysis of such projects as Apache , MySQL , Chromium . Why we choose such applications, I think it is clear. Everyone knows about them and nobody cares what horrors can be found in the student work of Vasya. However, sometimes we trust those projects that just happened to be on hand. Some such projects leave heavy impressions in my thin and vulnerable soul. This time we checked the Intel® Energy Checker SDK (IEC SDK).

The Intel Energy Checker SDK is a small C project containing just 74500 lines of code. For comparison, the size of the WinMerge project is 186,000 lines of code, and the size of the Miranda IM project with plug-ins is about 950,000 lines of code.

By the way, while we are at the beginning of the article, try to guess how many ' goto ' operators can be in this project. Guess the number? If yes, then continue.

In general, it is one of the small, unknown to the general public, projects. You will look at the code of such a project, and the following analogy arises. You can walk for a long time on famous good streets and hardly see puddles. But it is necessary to turn off the street and look into the courtyard, as a puddle is not only immediately located, it is not clear how you can walk without wetting your feet.

IEC SDK can be quite attributed to the projects, considering the text of which, the word “ govnokod ” pops up in my head. I will not say that the code is terrible, it is much worse. But see for yourself. In the whole project there are 247 functions. Say a little? Of course, not enough. But the size of some of them is shocking. Four functions generally have a size of more than 2000 lines:
')
V553 The length of 'pl_open' function's body is more than 2000 lines long. You should consider refactoring the code. pl_csv_logger productivity_link.c 379

V553 The length of 'pl_attach' function's body is more than 2000 lines long. You should consider refactoring the code. pl_csv_logger productivity_link.c 9434

V553 The length of the main function is more than 2000 lines long. You should consider refactoring the code. cluster_energy_efficiency cee.c 97

V553 The length of the main function is more than 2000 lines long. You should consider refactoring the code. pl2ganglia pl2ganglia.c 105

Indicative and here is a way to get the length of the directory name:

#define PL_FOLDER_STRING "C:\\productivity_link" #define PL_PATH_SEPARATOR_STRING "\\" #define PL_APPLICATION_NAME_SEPARATOR_STRING "_" ... pl_root_name_length = strlen(PL_FOLDER_STRING); pl_root_name_length += strlen(PL_PATH_SEPARATOR_STRING); pl_root_name_length += application_name_length; pl_root_name_length += strlen(PL_APPLICATION_NAME_SEPARATOR_STRING); pl_root_name_length += PL_UUID_MAX_CHARS; pl_root_name_length += strlen(PL_PATH_SEPARATOR_STRING); 

I understand that this place is not critical for speed and it makes no sense to optimize the calculation of the length of the lines. But just for the love of art, one could have such a macro "# define STRLEN (s) (sizeof (s) / sizeof (* s) - 1)" for this purpose). Even more my sense of beauty suffers from the fact that I see a line containing "C: \\". I am concerned about these macros:

 #define PL_INI_WINDOWS_FOLDER "C:\\productivity_link" #define PL_INI_WINDOWS_LC_FOLDER "c:\\productivity_link" #define PLH_FOLDER_SEARCH _T("C:\\productivity_link\\*") 

However, this code does what is needed and we will not focus on the programming style. The number of errors that PVS-Studio found in a project of such a small size is much more scary. It should be borne in mind that not 74,000 lines of code have been verified. About a third of the code is intended for LINUX / SOLARIS / MACOSX and is located in #ifdef # endif code branches that were not checked. The impassable forest from # ifdef / # endif is a separate topic, but I promised not to talk about code design anymore. Those interested can take a look at the source codes themselves.

The IEC SDK compiles a heterogeneous bunch of errors that can be made by operating with low-level arrays. However, errors of this kind are very typical of the C language.

There is a code that accesses memory outside the array:

V557 Array overrun is possible. The '255' index is pointing beyond array bound. pl2ganglia pl2ganglia.c 1114

 #define PL_MAX_PATH 255 #define PL2GANFLIA_COUNTER_MAX_LENGTH PL_MAX_PATH char name[PL_MAX_PATH]; int main(int argc, char *argv[]) { ... p->pl_counters_data[i].name[ PL2GANFLIA_COUNTER_MAX_LENGTH ] = '\0'; ... } 

We are dealing with a classic defect of writing a terminal zero outside the array. It should be:

 p->pl_counters_data[i].name[ PL2GANFLIA_COUNTER_MAX_LENGTH - 1 ] = '\0'; 

There are errors of zeroing structures.

V568 It's odd that the operator of the sizeof () operator is the '& file_data' expression. pl_csv_logger productivity_link_helper.c 1667

V568 It's odd that the operator of the sizeof () operator is the '& file_data' expression. pl_csv_logger productivity_link_helper.c 1831

V512 A call of the memset function will lead to underflow of the buffer pconfig. pl_csv_logger productivity_link_helper.c 1806

An example of such incorrect zeroing:

 int plh_read_pl_folder(PPLH_PL_FOLDER_INFO pconfig) { ... WIN32_FIND_DATA file_data; ... memset( &file_data, 0, sizeof(&file_data) ); ... } 

The job of finding files will go badly if you use the WIN32_FIND_DATA structure with garbage inside. However, there is a suspicion that the Windows version of this piece of programmer's art is of little interest to anyone. For example, the code is compiled in the “Use Unicode Character Set” mode, although it is not designed for this until the end. Apparently, no one thought. We created a project for Visual Studio, and the default setting "Character Set" just sets the use of UNICODE.

As a result, we have c dozens of places where lines are only partially cleared. I will give only one piece of similar code:

V512 A call of the memset function will lead to underflow of the buffer (pl_cvt_buffer) '. pl_csv_logger productivity_link_helper.c 683

 #define PL_MAX_PATH 255 typedef WCHAR TCHAR, *PTCHAR; TCHAR pl_cvt_buffer[PL_MAX_PATH] = { '\0' }; int plh_read_pl_config_ini_file(...) { ... ZeroMemory( pl_cvt_buffer, PL_MAX_PATH ); ... } 

However, there are places where disabling UNICODE does not help. Here, instead of the text, something incomprehensible will be printed:

V576 Incorrect format. Consider the second argument of the wprintf function. Wchar_t type symbols is expected. producer producer.c 166

 int main(void) { ... char *p = NULL; ... #ifdef __PL_WINDOWS__ wprintf( _T("Using power link directory: %s\n"), p ); #endif // __PL_WINDOWS__ ... } 

I will explain. The wprintf function expects a string of type “wchar_t *”, and it will be passed to it strings of type “char *”.

There are other bugs, and small bloopers like this:

V571 Recurring check. The 'if (ret == PL_FAILURE)' condition was already verified in line 1008. pl_csv_logger pl_csv_logger.c 1009

 if(ret == PL_FAILURE) { if(ret == PL_FAILURE) { pl_csv_logger_error( PL_CSV_LOGGER_ERROR_UNABLE_TO_READ_PL ); 

I do not see any point in trying to enumerate all the noticed flaws. If someone wants, I can provide a key for checking the project and analyzing the messages. I will also send an error description to the SDK authors.

And now dessert

Remember, at the beginning, I asked to guess how many 'goto' operators can be found in the project. So, I think your number is far from the truth. In total, the project uses 1198 goto operators. One goto statement with 60 lines of code. And I thought that this style has already sunk into oblivion.

Conclusion

And what exactly did the author want to say with this article? Intel URGENTLY needs to look closely at PVS-Studio. :-)