To get a trial key
fill out the form below
Team License (standard version)
Enterprise License (extended version)
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Checking Intel Energy Checker SDK (IEC …

Checking Intel Energy Checker SDK (IEC SDK) with PVS-Studio

July 28, 2011
Author:

Recently, while telling you about check of another project, I have constantly repeated that it is a very quality code and there are almost no errors in it. A good example is analysis of such projects as Apache, MySQL and Chromium. I think you understand why we choose such applications for analysis. They are known to all of us while nobody wants to hear about horrible things found in the diploma design of student Jack. But sometimes we check projects that just come to hand. Some of such projects leave painful impressions in my delicate and vulnerable soul. This time we checked Intel(R) Energy Checker SDK (IEC SDK).

0106_Intel_developers/image1.png

Intel Energy Checker SDK is a small C project containing just 74500 lines of code. Compare this number to the WinMerge project's size of 186 000 lines or size of the Miranda IM project together with the plugins (about 950 000 lines).

0106_Intel_developers/image2.png

By the way, while we are just in the beginning of the article, try to guess how many 'goto' operators there are in this project. Have you thought of a number? If yes, then we go on.

All in all, this is one of those small projects unknown to a wide audience. When I look at a project of that kind, the following analogy occurs to me. You may walk on good familiar streets for a long time without seeing any puddles. But as you make a turn and look into a yard, you just not only see a puddle but a puddle so big that you don't understand how you can pass it without getting your feet wet.

I wouldn't say that the code is horrible, there are cases much worse. But look at it yourself. There are only 247 functions in the whole project. You will say it's few, won't you? Of course, it's few. But the size of some of them embarrasses me. Four functions have size of more than 2000 lines each:

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 'main' function's body is more than 2000 lines long. You should consider refactoring the code. cluster_energy_efficiency cee.c 97

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

The following method of getting the length of a directory's name is also significant:

#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 fragment is not crucial to speed and there's no reason to optimize the string length calculation. But just from the mere love to art, the programmer could have created a macro "#define STRLEN(s) (sizeof(s) / sizeof(*s) - 1)". My sense of beauty suffers even more because of strings containing "C:\\". The following macros alert me:

#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 it should and we won't focus our attention on the programming style. It is the number of errors found by PVS-Studio in a project of such a small size - this is what scares me most. And keep in mind that far not all the 74000 code lines were checked. About one third of the code is intended for LINUX/SOLARIS/MACOSX and is stored in #ifdef/#endif code branches that were not checked. The impassible wood of #ifdef/#endif's is just a different story but I promised not to speak of code design anymore. If you wish, you may look through the source codes yourselves.

The code of IEC SDK has collected a diverse bunch of mistakes one can make when handling arrays at the low level. However, mistakes of this kind are very typical of the C language.

There is code addressing memory outside an 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';
  ...
}

Here we deal with a typical defect of writing the terminal zero outside the array. The code must look this way:

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

There are errors of structure clearing.

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

V568 It's odd that the argument of 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

Here is an example of such incorrect emptying:

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

The code intended for file search will work badly when you use the WIN32_FIND_DATA structure with rubbish inside it. But I suspect that hardly anybody is interested in the Windows version of this work of programming art. For instance, the code compiles in the "Use Unicode Character Set" mode although it is not fully intended for this. It seems that nobody ever thought of this - they just created the project for Visual Studio and the "Character Set" setting by default defines use of UNICODE.

As a result, there are a dozen of fragments where strings are cleared only partially. I'll cite only one sample of such 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
  );
  ...
}

Well, there are, however, places where disabling UNICODE won't help. Something strange will be printed here instead of text:

V576 Incorrect format. Consider checking the second actual argument of the 'wprintf' function. The pointer to string of 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__
  ...
}

Let me explain this. The wprintf function waits for a string of the "wchar_t *" type while it is a string of the "char *" type that will be passed to it.

There are other errors and small defects similar to this one:

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 see no reason in enumerating all the defects we have found. If somebody of you wants, I may give you a registration key to check the project and study the messages. I will also send error descriptions to SDK's authors.

And here you are the dessert

Do you remember I asked you to think of a number of 'goto' operators found in the project? So, I think your number is far from the truth. There are 1198 goto operators altogether in the project, i.e. one goto operator for every 60 code lines. And I thought that style had been forgotten for a long time.

Summary

Well, and what did the author actually want to say by writing this article? Intel URGENTLY needs to pay the greatest attention to PVS-Studio. :-)

Testimonial from Jamel Tayeb, developer of IEC SDK

Dear Andrey and PVS-Studio team,

I first want to re-iterate my thank you for bringing to my attention my code's imperfections. Using your tool was really a great opportunity to improve the code's quality.

Error humanum est, does apply to all of use, and especially to me in this case! I wrote almost all of the Energy Checker code so I think that it could be interesting to share my experience with your tool with your potential customers. Beside the great insight it provides - I will come back to this later on - I really appreciated the good integration into Microsoft Visual Studio 2005. This is important to me because I could use the software as-is, "out of the box" and work to remove my bugs immediately. On the downside, I would essentially quote the speed of the analysis. I believe that your company is working on improving it, so I am confident that the use of PVS-Studio will be even smoother in the future.

Now let's get to the point. PVS-Studio slapped me on the hand for my mistakes. And this is what I expect from such software. Sure our code runs OK, with good performance and fulfilling its role (until the next bug report, of course). But in many cases, I was feeling guilty that that some code section just work because I was lucky. Applying the correction, makes them work because they are correct now. And when such mistake happens to be in a macro, it is a great pleasure to see the error count drop down drastically just after an incremental analysis. Another mistake of mines that struck me was that copy-and-past can be a false friend. It helps to speed up crafting a new section of code, but you need to be very vigilant doing so. Obviously in some cases, I was not enough vigilant. PVS-Studio woke me up.

Again, thank you for your feedback, your software's insight and for letting me try PVS-Studio.

Jamel Tayeb, IEC SDK

Popular related articles
The way static analyzers fight against false positives, and why they do it

Date: 03.20.2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…
The Evil within the Comparison Functions

Date: 05.19.2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
PVS-Studio ROI

Date: 01.30.2019

Author: Andrey Karpov

Occasionally, we're asked a question, what monetary value the company will receive from using PVS-Studio. We decided to draw up a response in the form of an article and provide tables, which will sho…
Static analysis as part of the development process in Unreal Engine

Date: 06.27.2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: 10.22.2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
The Ultimate Question of Programming, Refactoring, and Everything

Date: 04.14.2016

Author: Andrey Karpov

Yes, you've guessed correctly - the answer is "42". In this article you will find 42 recommendations about coding in C++ that can help a programmer avoid a lot of errors, save time and effort. The au…
The Last Line Effect

Date: 05.31.2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…
Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities

Date: 11.21.2018

Author: Andrey Karpov

A brief description of technologies used in the PVS-Studio tool, which let us effectively detect a large number of error patterns and potential vulnerabilities. The article describes the implementati…
PVS-Studio for Java

Date: 01.17.2019

Author: Andrey Karpov

In the seventh version of the PVS-Studio static analyzer, we added support of the Java language. It's time for a brief story of how we've started making support of the Java language, how far we've co…
Free PVS-Studio for those who develops open source projects

Date: 12.22.2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…

Comments (0)

Next comments

This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept