# PVS\-Studio in the clouds: CircleCI This is a new piece of our series of articles about using the PVS\-Studio static analyzer with cloud CI systems\. Today we are going to look at another service, CircleCI\. We'll take the Kodi media player application as a test project and see if we can find any interesting bugs in its source code\. > To get current information about it follow the updated documentation page "\[Using with CircleCI\]\(https://pvs\-studio\.com/en/docs/manual/0054/\)"\\\. ![0676_CircleCI/image1.png](https://import.viva64.com/docx/blog/0676_CircleCI/image1.png) **Note\.** The previous articles on integrating PVS\-Studio with cloud CI systems: * [PVS\-Studio in the clouds: Azure DevOps](https://pvs-studio.com/en/blog/posts/csharp/0670/) * [PVS\-Studio in the clouds \- Running the Analysis on Travis CI](https://pvs-studio.com/en/blog/posts/cpp/0636/) Before setting up the work environment and examining the analysis report, I'd like to say a few words about the software we are going to use and check\. [CircleCI](https://circleci.com/) is a cloud CI service for automated software building, testing, and deployment\. It supports project building both in containers and on virtual machines on Windows, Linux, and macOS\. [Kodi](https://en.wikipedia.org/wiki/Kodi_(software)) is a free and open\-source cross\-platform media player application\. It allows users to play and view most streaming media, such as videos, music, podcasts, and videos from the Internet, as well as all common digital media files from local and network storage media\. It supports the use of themes and skins and functionality extensions through plugins\. Kodi is available for Windows, Linux, macOS, and Android\. [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) is a static analyzer for detecting bugs and potential vulnerabilities in the source code of applications written in C, C\+\+, C\#, and Java\. The analyzer runs on Windows, Linux, and macOS\. ## Setting up First we need to go to the [CircleCI](https://circleci.com/) main page and click "Sign Up" ![0676_CircleCI/image2.png](https://import.viva64.com/docx/blog/0676_CircleCI/image2.png) On the next page, we are offered to authorize with a GitHub or Bitbucket account\. We choose GitHub and get to the CircleCI authorization page\. ![0676_CircleCI/image3.png](https://import.viva64.com/docx/blog/0676_CircleCI/image3.png) After authorizing the application \(by clicking the green button "Authorize circleci"\), we are redirected to the "Welcome to CircleCI\!" page: ![0676_CircleCI/image4.png](https://import.viva64.com/docx/blog/0676_CircleCI/image4.png) Here we can specify right away which projects we want CircleCI to build\. We tick our repository and click "Follow"\. After adding the repository, CircleCI will automatically start the build process, but since we don't have a configuration file in our repository yet, the build job will be aborted with an error message\. ![0676_CircleCI/image5.png](https://import.viva64.com/docx/blog/0676_CircleCI/image5.png) Before adding a configuration file, we need to add a couple of variables containing analyzer license data\. To do that, we click "Settings" on the left sidebar, select "Projects" in the "ORGANIZATION" section, and click the cogwheel button to the right of our project's name\. A settings window will appear\. ![0676_CircleCI/image6.png](https://import.viva64.com/docx/blog/0676_CircleCI/image6.png) We go to the "Environment Variables" page\. Here we create two variables, _PVS\_USERNAME_ and _PVS\_KEY_, which contain the username and analyzer license key\. ![0676_CircleCI/image7.png](https://import.viva64.com/docx/blog/0676_CircleCI/image7.png) When starting the build, CircleCI reads the job configuration from the file stored in the repository at \.circleci/config\.yml\. Let's add it\. First we need to specify the image of the virtual machine that the analyzer will be running on\. The complete list of images is available [here](https://circleci.com/docs/2.0/configuration-reference/)\. ```cpp version: 2 jobs: build: machine: image: ubuntu-1604:201903-01 ``` Next, we add the necessary repositories to apt and install the project's dependencies: ```cpp steps: - checkout - run: sudo -- sh -c " add-apt-repository -y ppa:team-xbmc/xbmc-ppa-build-depends && add-apt-repository -y ppa:wsnipex/vaapi && add-apt-repository -y ppa:pulse-eight/libcec && apt-get update" - run: sudo apt-get install -y automake autopoint build-essential cmake curl default-jre gawk gdb gdc gettext git-core gperf libasound2-dev libass-dev libbluray-dev libbz2-dev libcap-dev libcdio-dev libcec4-dev libcrossguid-dev libcurl3 libcurl4-openssl-dev libdbus-1-dev libegl1-mesa-dev libfmt3-dev libfontconfig-dev libfreetype6-dev libfribidi-dev libfstrcmp-dev libgif-dev libgl1-mesa-dev libglu1-mesa-dev libiso9660-dev libjpeg-dev liblcms2-dev libltdl-dev liblzo2-dev libmicrohttpd-dev libmysqlclient-dev libnfs-dev libpcre3-dev libplist-dev libpng-dev libpulse-dev libsmbclient-dev libsqlite3-dev libssl-dev libtag1-dev libtinyxml-dev libtool libudev-dev libusb-dev libva-dev libvdpau-dev libxml2-dev libxmu-dev libxrandr-dev libxrender-dev libxslt1-dev libxt-dev mesa-utils nasm pmount python-dev python-imaging python-sqlite rapidjson-dev swig unzip uuid-dev yasm zip zlib1g-dev wget ``` Adding the PVS\-Studio repository and installing the analyzer: ```cpp - run: wget -q -O - https://files.pvs-studio.com/etc/pubkey.txt | sudo apt-key add - && sudo wget -O /etc/apt/sources.list.d/viva64.list https://files.pvs-studio.com/etc/viva64.list - run: sudo -- sh -c "apt-get update && apt-get install pvs-studio -y" ``` Then we are building the dependencies: ```cpp - run: sudo make -C tools/depends/target/flatbuffers PREFIX=/usr/local ``` After that, we generate Makefiles in the build directory: ```cpp - run: mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Debug .. ``` The next step is setting up and starting an analysis of the project\. First we create an analyzer license file\. Another command will start tracing the project build by the compiler\. The next command following the tracing runs the analysis as such\. If you use a demo version of PVS\-Studio, launch it with the parameter: _\-\-disableLicenseExpirationCheck_\. The final command converts the analyzer's report file into an html report: ```cpp - run: pvs-studio-analyzer credentials -o PVS.lic ${PVS_USER} ${PVS_KEY} - run: pvs-studio-analyzer trace -- make -j2 -C build/ - run: pvs-studio-analyzer analyze -j2 -l PVS.lic -o PVS-Studio.log --disableLicenseExpirationCheck - run: plog-converter -t html -o PVS-Studio.html PVS-Studio.log ``` Once the tests are over, we save the reports: ```cpp - run: mkdir PVS_Result && cp PVS-Studio.* ./PVS_Result/ - store_artifacts: path: ./PVS_Result ``` Here's the complete text of the \.circleci/config\.yml file: ```cpp version: 2.1 jobs: build: machine: image: ubuntu-1604:201903-01 steps: - checkout - run: sudo -- sh -c " add-apt-repository -y ppa:team-xbmc/xbmc-ppa-build-depends && add-apt-repository -y ppa:wsnipex/vaapi && add-apt-repository -y ppa:pulse-eight/libcec && apt-get update" - run: sudo apt-get install -y automake autopoint build-essential cmake curl default-jre gawk gdb gdc gettext git-core gperf libasound2-dev libass-dev libbluray-dev libbz2-dev libcap-dev libcdio-dev libcec4-dev libcrossguid-dev libcurl3 libcurl4-openssl-dev libdbus-1-dev libegl1-mesa-dev libfmt3-dev libfontconfig-dev libfreetype6-dev libfribidi-dev libfstrcmp-dev libgif-dev libgl1-mesa-dev libglu1-mesa-dev libiso9660-dev libjpeg-dev liblcms2-dev libltdl-dev liblzo2-dev libmicrohttpd-dev libmysqlclient-dev libnfs-dev libpcre3-dev libplist-dev libpng-dev libpulse-dev libsmbclient-dev libsqlite3-dev libssl-dev libtag1-dev libtinyxml-dev libtool libudev-dev libusb-dev libva-dev libvdpau-dev libxml2-dev libxmu-dev libxrandr-dev libxrender-dev libxslt1-dev libxt-dev mesa-utils nasm pmount python-dev python-imaging python-sqlite rapidjson-dev swig unzip uuid-dev yasm zip zlib1g-dev wget - run: wget -q -O - https://files.pvs-studio.com/etc/pubkey.txt | sudo apt-key add – && sudo wget -O /etc/apt/sources.list.d/viva64.list https://files.pvs-studio.com/etc/viva64.list - run: sudo -- sh -c "apt-get update && apt-get install pvs-studio -y" - run: sudo make -C tools/depends/target/flatbuffers PREFIX=/usr/local - run: mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Debug .. - run: pvs-studio-analyzer credentials -o PVS.lic ${PVS_USER} ${PVS_KEY} - run: pvs-studio-analyzer trace -- make -j2 -C build/ - run: pvs-studio-analyzer analyze -j2 -l PVS.lic -o PVS-Studio.log --disableLicenseExpirationCheck - run: plog-converter -t html -o PVS-Studio.html PVS-Studio.log - run: mkdir PVS_Result && cp PVS-Studio.* ./PVS_Result/ - store_artifacts: path: ./PVS_Result ``` Once this file is uploaded to the repository, CircleCI will automatically start the build\. ![0676_CircleCI/image8.png](https://import.viva64.com/docx/blog/0676_CircleCI/image8.png) Once the job is finished, the files with the analysis results can be downloaded on the "Artifacts" tab\. ![0676_CircleCI/image9.png](https://import.viva64.com/docx/blog/0676_CircleCI/image9.png) ## Analysis results OK, now let's take a look at some of the warnings output by the analyzer\. **PVS\-Studio warning:** [V504](https://pvs-studio.com/en/docs/warnings/v504/) It is highly probable that the semicolon ';' is missing after 'return' keyword\. AdvancedSettings\.cpp:1476 ```cpp void CAdvancedSettings::SetExtraArtwork(const TiXmlElement* arttypes, std::vector& artworkMap) { if (!arttypes) return artworkMap.clear(); const TiXmlNode* arttype = arttypes->FirstChild("arttype"); .... } ``` The code formatting suggests the following execution logic: * if _arttypes_ is a null pointer, the method returns; * if _arttypes_ is a non\-null pointer, the _artworkMap_ vector gets cleared and some actions are then performed\. But the missing ';' character breaks it all, and the actual execution logic is as follows: * if _arttypes_ is a null pointer, the _artworkMap_ vector gets cleared and the method returns; * if _arttypes_ is a non\-null pointer, the program executes whatever actions come next but the _artworkMap_ vector doesn't get cleared\. To cut a long story short, this situation does look like a bug\. After all, you hardly expect anyone to write expressions like _return artworkMap\.clear\(\);_ :\)\. **PVS\-Studio warnings:** * [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'lastsector' is always false\. udf25\.cpp:636 * [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'lastsector' is always false\. udf25\.cpp:644 * [V571](https://pvs-studio.com/en/docs/warnings/v571/) Recurring check\. The 'if \(lastsector\)' condition was already verified in line 636\. udf25\.cpp:644 ```cpp int udf25::UDFGetAVDP( struct avdp_t *avdp) { .... uint32_t lastsector; .... lastsector = 0; // <= .... for(;;) { .... if( lastsector ) { // <= V547 lbnum = lastsector; terminate = 1; } else { //! @todo Find last sector of the disc (this is optional). if( lastsector ) // <= V547 lbnum = lastsector - 256; else return 0; } } .... } ``` Note the spots marked with _// <\=_\. The _lastsector_ variable is assigned the value 0 and then used as a conditional expression in two _if_ statements\. Since the value doesn't change either in the loop or between the assignments, control will never get into _then_ branches of both _if_ statements\. However, it could also mean that the developers simply haven't implemented the intended functionality yet \(note the _@todo_ remark\)\. By the way, as you probably noticed, this snippet triggered three warnings at once\. But even that many warnings for one piece of code wouldn't look convincing enough to some users, and they would keep believing that the analyzer is wrong\.\.\. This aspect is discussed in detail in a post by one of my teammates: "[One day from PVS\-Studio user support](https://pvs-studio.com/en/blog/posts/cpp/0671/)" :\)\. **PVS\-Studio warning:** [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'values\.size\(\) \!\= 2' is always false\. GUIControlSettings\.cpp:1174 ```cpp bool CGUIControlRangeSetting::OnClick() { .... std::vector values; SettingConstPtr listDefintion = settingList->GetDefinition(); switch (listDefintion->GetType()) { case SettingType::Integer: values.push_back(m_pSlider-> GetIntValue(CGUISliderControl::RangeSelectorLower)); values.push_back(m_pSlider-> GetIntValue(CGUISliderControl::RangeSelectorUpper)); break; case SettingType::Number: values.push_back(m_pSlider-> GetFloatValue(CGUISliderControl::RangeSelectorLower)); values.push_back(m_pSlider-> GetFloatValue(CGUISliderControl::RangeSelectorUpper)); break; default: return false; } if (values.size() != 2) return false; SetValid(CSettingUtils::SetList(settingList, values)); return IsValid(); } ``` The _values\.size\(\)_ _\!\=_ _2_ check is redundant here since this conditional expression will always evaluate to _false_\. Indeed, if execution enters one of the _case_ branches of the _switch_ statement, two elements will be added to the vector, and since it was initially empty, its size will naturally become equal to 2; otherwise \(i\.e\. if the _default_ branch is executed\), the method will return\. **PVS\-Studio warning:** [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'prio \=\= 0x7fffffff' is always true\. DBusReserve\.cpp:57 ```cpp bool CDBusReserve::AcquireDevice(const std::string& device) { .... int prio = INT_MAX; .... res = dbus_bus_request_name( m_conn, service.c_str(), DBUS_NAME_FLAG_DO_NOT_QUEUE | (prio == INT_MAX ? 0 : DBUS_NAME_FLAG_ALLOW_REPLACEMENT), // <= error); .... } ``` The _prio_ variable is initialized to the _INT\_MAX_ value and then used as an operand of the ternary operator in the _prio \=\= INT\_MAX_ comparison, though its value doesn't change after the initialization\. It means the _prio \=\= INT\_MAX_ expression is _true_ and the ternary operator will always return 0\. **PVS\-Studio warnings:** * [V575](https://pvs-studio.com/en/docs/warnings/v575/) The potential null pointer is passed into 'memcpy' function\. Inspect the first argument\. Check lines: 39, 38\. DVDOverlayImage\.h:39 * [V575](https://pvs-studio.com/en/docs/warnings/v575/) The potential null pointer is passed into 'memcpy' function\. Inspect the first argument\. Check lines: 44, 43\. DVDOverlayImage\.h:44 ```cpp CDVDOverlayImage(const CDVDOverlayImage& src) : CDVDOverlay(src) { Data = (uint8_t*)malloc(src.linesize * src.height); memcpy(data, src.data, src.linesize * src.height); // <= if(src.palette) { palette = (uint32_t*)malloc(src.palette_colors * 4); memcpy(palette, src.palette, src.palette_colors * 4); // <= } .... } ``` Both warnings have the same pattern: a pointer returned by the _malloc_ function is used further in the _memcpy_ function without being checked for _NULL_ first\. Some may argue that _malloc_ will never return a null pointer, and if it does, it would be better for the application to crash\. It's a subject of a separate discussion, but whatever your opinion is, I recommend reading this post by my teammate: "[Why it is important to check what the malloc function returned](https://pvs-studio.com/en/blog/posts/cpp/0558/)"\. If you wish, you can customize the analyzer so that it doesn't assume that _malloc_ could return a null pointer – this will keep it from outputting this type of warnings\. More details can be found [here](https://pvs-studio.com/en/docs/manual/0040/)\. **PVS\-Studio warning:** [V522](https://pvs-studio.com/en/docs/warnings/v522/) There might be dereferencing of a potential null pointer 'entry'\. Check lines: 985, 981\. emu\_msvcrt\.cpp:985 ```cpp struct dirent *dll_readdir(DIR *dirp) { .... struct dirent *entry = NULL; entry = (dirent*) malloc(sizeof(*entry)); if (dirData->curr_index < dirData->items.Size() + 2) { if (dirData->curr_index == 0) strncpy(entry->d_name, ".\0", 2); .... } ``` This example is similar to the previous one\. The pointer returned by the _malloc_ function is stored to the _entry_ variable, and this variable is then used without a prior null check \(_entry\-\>d\_name_\)\. **PVS\-Studio warning:** [V773](https://pvs-studio.com/en/docs/warnings/v773/) Visibility scope of the 'progressHandler' pointer was exited without releasing the memory\. A memory leak is possible\. PVRGUIChannelIconUpdater\.cpp:94 ```cpp void CPVRGUIChannelIconUpdater::SearchAndUpdateMissingChannelIcons() const { .... CPVRGUIProgressHandler* progressHandler = new CPVRGUIProgressHandler(g_localizeStrings.Get(19286)); for (const auto& group : m_groups) { const std::vector members = group->GetMembers(); int channelIndex = 0; for (const auto& member : members) { progressHandler->UpdateProgress(member.channel->ChannelName(), channelIndex++, members.size()); .... } progressHandler->DestroyProgress(); } ``` The value of the _progressHandler_ pointer was returned by the _new_ operator\. But there is no _delete_ operator for this pointer\. This means a memory leak\. **PVS\-Studio warning:** [V557](https://pvs-studio.com/en/docs/warnings/v557/) Array overrun is possible\. The 'idx' index is pointing beyond array bound\. PlayerCoreFactory\.cpp:240 ```cpp std::vector m_vecPlayerConfigs; bool CPlayerCoreFactory::PlaysVideo(const std::string& player) const { CSingleLock lock(m_section); size_t idx = GetPlayerIndex(player); if (m_vecPlayerConfigs.empty() || idx > m_vecPlayerConfigs.size()) return false; return m_vecPlayerConfigs[idx]->m_bPlaysVideo; } ``` The _if_ statement restricts the _m\_vecPlayerConfigs_ vector's size to a certain range by having the method return if the size\-checking condition is true\. As a result, when execution reaches the last _return_ statement, the _m\_vecPlayerConfigs_ vector's size will be within the specified range, \[1; idx\]\. But a couple of lines later, the program is indexing the vector at _idx_: _m\_vecPlayerConfigs\[idx\]\-\>m\_bPlaysVideo_\. It means that if _idx_ is equal to the vector's size, we'll be indexing beyond the valid range\. Let's wind up this article with a couple of examples from the code of the **Platinum** library\. **PVS\-Studio warning:** [V542](https://pvs-studio.com/en/docs/warnings/v542/) Consider inspecting an odd type cast: 'bool' to 'char \*'\. PltCtrlPoint\.cpp:1617 ```cpp NPT_Result PLT_CtrlPoint::ProcessSubscribeResponse(...) { .... bool subscription = (request.GetMethod().ToUppercase() == "SUBSCRIBE"); .... NPT_String prefix = NPT_String::Format(" PLT_CtrlPoint::ProcessSubscribeResponse %ubscribe for service \"%s\" (result = %d, status code = %d)", (const char*)subscription?"S":"Uns", // <= (const char*)service->GetServiceID(), res, response?response->GetStatusCode():0); .... } ``` The developers have had wrong assumptions about the operations' precedence\. What gets cast to _const char\*_ is not the result returned by the ternary operator \(_subscription ? "S" : "Uns"_\) but the _subscription_ variable\. This looks strange, at the very least\. **PVS\-Studio warning:** [V560](https://pvs-studio.com/en/docs/warnings/v560/) A part of conditional expression is always false: c \=\= '\\t'\. NptUtils\.cpp:863 ```cpp NPT_Result NPT_ParseMimeParameters(....) { .... case NPT_MIME_PARAMETER_PARSER_STATE_NEED_EQUALS: if (c < ' ') return NPT_ERROR_INVALID_SYNTAX; // END or CTLs are invalid if (c == ' ' || c == '\t') continue; // ignore leading whitespace .... } ``` The code of the space character is 0x20, and the code of the tab character is 0x09\. Therefore, the _c \=\= '\\t'_ subexpression will always evaluate to _false_ as this case is already covered by the _c < ' '_ check \(which, if true, will cause the function to return\)\. ## Conclusion As this article demonstrates, we successfully set up an analysis by PVS\-Studio on yet another CI system \(CircleCI\)\. I invite you to [download and try](https://pvs-studio.com/en/pvs-studio/download/) the analyzer on your own project\. If you have any questions about the setting\-up or use of PVS\-Studio, don't hesitate to [contact us](https://pvs-studio.com/en/about-feedback/) – we'll be glad to help\. And, of course, we wish you bugless code\. :\)