We continue to make using PVS-Studio more convenient. Our analyzer is now available in Chocolatey, a package manager for Windows. We believe that this will facilitate the deployment of PVS-Studio, in particular, in cloud services. In order not to go far, let's check the source code of the same Chocolatey. Azure DevOps will act as a CI system.
Here is a list of our other articles on cloud integration:
I advise you to pay attention to the first article about integration with Azure DevOps, since in this case some points are omitted so as not to be duplicated.
So, the heroes of this article:
is a static code analysis tool designed to detect errors and potential vulnerabilities in programs written in C, C++, C# and Java. Works on 64-bit systems on Windows, Linux and macOS, and can analyze code designed for 32-bit, 64-bit and embedded ARM platforms. If this is your first time trying static code analysis to check your projects, we recommend that you familiarize yourself with about how to quickly view the most interesting PVS-Studio warnings and evaluate the capabilities of this tool.
- a set of cloud services that collectively cover the entire development process. This platform includes tools such as Azure Pipelines, Azure Boards, Azure Artifacts, Azure Repos, Azure Test Plans to speed up the process of creating software and improve its quality.
is an open source package manager for Windows. The goal of the project is to automate the entire software life cycle from installation to upgrade and removal in Windows operating systems.
About using Chocolatey
To see how to install the package manager itself, you can follow this . Complete documentation for installing the analyzer is available at See "Installing Using the Chocolatey Package Manager". I will briefly repeat some points from there.
Command to install the latest version of the analyzer:
choco install pvs-studioThe command to install a specific version of the PVS-Studio package:
choco install pvs-studio --version=7.05.35617.2075By default, only the core of the analyzer is installed - the Core component. All other flags (Standalone, JavaCore, IDEA, MSVS2010, MSVS2012, MSVS2013, MSVS2015, MSVS2017, MSVS2019) can be passed using --package-parameters.
An example of a command that will install an analyzer with a plugin for Visual Studio 2019:
choco install pvs-studio --package-parameters="'/MSVS2019'"Now let's look at an example of a convenient use of the analyzer for Azure DevOps.
Setting
I remind you that there is a separate . Our setup will immediately begin by writing a configuration file.
First, let's set up a launch trigger, specifying that we launch only for changes in master branch:
trigger:
- masterNext, we need to select a virtual machine. For now, this will be a Microsoft-hosted agent with Windows Server 2019 and Visual Studio 2019:
pool:
vmImage: 'windows-latest'Let's move on to the body of the configuration file (block steps). Despite the fact that arbitrary software cannot be installed in a virtual machine, I did not add a Docker container. We can add Chocolatey as an extension for Azure DevOps. To do this, let's go to . We press Get it free. Further, if you are already authorized, simply select your account, and if not, then we do the same after authorization.
Here you need to choose where we will add the extension, and click the button install.
After a successful installation, click Proceed to organization:
You can now see the template for the Chocolatey task in the window tasks when editing a configuration file azure-pipelines.yml:
Click on Chocolatey and see the list of fields:
Here we need to choose install in the field with commands. IN Nuspec File Name specify the name of the required package - pvs-studio. If you do not specify the version, the latest one will be installed, which suits us completely. Let's press the button add and we will see the generated task in the configuration file.
steps:
- task: ChocolateyCommand@0
inputs:
command: 'install'
installPackageId: 'pvs-studio'Next, let's move on to the main part of our file:
- task: CmdLine@2
inputs:
script: Now we need to create a file with the analyzer license. Here PVSNAME и PVSKEY – names of variables whose values we specify in the settings. They will store the PVS-Studio login and license key. To set their values, open the menu Variables->New variable. Let's create variables PVSNAME for login and PVSKEY for the parser key. Don't forget to tick Keep this value secret for PVSKEY. Command code:
сall "C:Program Files (x86)PVS-StudioPVS-Studio_Cmd.exe" credentials
–u $(PVSNAME) –n $(PVSKEY)Let's build the project using the bat-file located in the repository:
сall build.batLet's create a folder where the files with the results of the analyzer will be located:
сall mkdir PVSTestResultsLet's start the project analysis:
сall "C:Program Files (x86)PVS-StudioPVS-Studio_Cmd.exe"
–t .srcchocolatey.sln –o .PVSTestResultsChoco.plog We convert our report to html format using the PlogСonverter utility:
сall "C:Program Files (x86)PVS-StudioPlogConverter.exe"
–t html –o PVSTestResults .PVSTestResultsChoco.plogNow you need to create a task in order to be able to upload the report.
- task: PublishBuildArtifacts@1
inputs:
pathToPublish: PVSTestResults
artifactName: PVSTestResults
condition: always()The complete configuration file looks like this:
trigger:
- master
pool:
vmImage: 'windows-latest'
steps:
- task: ChocolateyCommand@0
inputs:
command: 'install'
installPackageId: 'pvs-studio'
- task: CmdLine@2
inputs:
script: |
call "C:Program Files (x86)PVS-StudioPVS-Studio_Cmd.exe"
credentials –u $(PVSNAME) –n $(PVSKEY)
call build.bat
call mkdir PVSTestResults
call "C:Program Files (x86)PVS-StudioPVS-Studio_Cmd.exe"
–t .srcchocolatey.sln –o .PVSTestResultsChoco.plog
call "C:Program Files (x86)PVS-StudioPlogConverter.exe"
–t html –o .PVSTestResults .PVSTestResultsChoco.plog
- task: PublishBuildArtifacts@1
inputs:
pathToPublish: PVSTestResults
artifactName: PVSTestResults
condition: always()Let's press Save->Save->Run to start the task. Download the report by going to the tasks tab.
The Chocolatey project contains a total of 37615 lines of C# code. Let's take a look at some of the bugs we found.
Validation Results
Warning N1
Analyzer warning: The 'Provider' variable is assigned to itself. CrytpoHashProviderSpecs.cs 38
public abstract class CrytpoHashProviderSpecsBase : TinySpec
{
....
protected CryptoHashProvider Provider;
....
public override void Context()
{
Provider = Provider = new CryptoHashProvider(FileSystem.Object);
}
}The analyzer detected the assignment of a variable to itself, which does not make sense. Most likely, in place of one of these variables there should be some other one. Well, or this is a typo, and the extra assignment can simply be deleted.
Warning N2
Analyzer warning: [CWE-480] The '&' operator evaluates both operands. Perhaps a short-circuit '&&' operator should be used instead. Platform.cs 64
public static PlatformType get_platform()
{
switch (Environment.OSVersion.Platform)
{
case PlatformID.MacOSX:
{
....
}
case PlatformID.Unix:
if(file_system.directory_exists("/Applications")
& file_system.directory_exists("/System")
& file_system.directory_exists("/Users")
& file_system.directory_exists("/Volumes"))
{
return PlatformType.Mac;
}
else
return PlatformType.Linux;
default:
return PlatformType.Windows;
}
}Operator difference & from operator && is that if the left side of the expression is false, then the right side will still be calculated, which in this case means extra method calls system.directory_exists.
In the reviewed fragment, this is a minor flaw. Yes, this condition can be optimized by replacing the & operator with the && operator, but, from a practical point of view, this does not affect anything. However, in other cases, the confusion between & and && can cause serious problems when the right side of the expression will work with incorrect/invalid values. For example, in our error collection, , there is such a case:
if ((k < nct) & (s[k] != 0.0))Even if the index k is invalid, it will be used to access an array element. This will result in an exception being thrown. IndexOutOfRangeException.
Warnings N3, N4
Analyzer warning: [CWE-571] Expression 'shortPrompt' is always true. InteractivePrompt.cs 101
Analyzer warning: [CWE-571] Expression 'shortPrompt' is always true. InteractivePrompt.cs 105
public static string
prompt_for_confirmation(.... bool shortPrompt = false, ....)
{
....
if (shortPrompt)
{
var choicePrompt = choice.is_equal_to(defaultChoice) //1
?
shortPrompt //2
?
"[[{0}]{1}]".format_with(choice.Substring(0, 1).ToUpperInvariant(), //3
choice.Substring(1,choice.Length - 1))
:
"[{0}]".format_with(choice.ToUpperInvariant()) //0
:
shortPrompt //4
?
"[{0}]{1}".format_with(choice.Substring(0,1).ToUpperInvariant(), //5
choice.Substring(1,choice.Length - 1))
:
choice; //0
....
}
....
}In this case, there is a strange logic of the ternary operator. Let us consider in more detail: if the condition marked by me with the number 1 is fulfilled, then we will go to condition 2, which is always true, which means line 3 will be executed. If condition 1 turns out to be false, then we will go to the line marked with the number 4, the condition in which is also always true, which means that line 5 will be executed. Thus, the conditions marked with comment 0 will never be met, which may not be exactly the logic of work that the programmer was counting on.
Warning N5
Analyzer warning: [CWE-783] Perhaps the '?:' operator works in a different way than it was expected. Its priority is lower than the priority of other operators in its condition. Options.cs 1019
private static string GetArgumentName (...., string description)
{
string[] nameStart;
if (maxIndex == 1)
{
nameStart = new string[]{"{0:", "{"};
}
else
{
nameStart = new string[]{"{" + index + ":"};
}
for (int i = 0; i < nameStart.Length; ++i)
{
int start, j = 0;
do
{
start = description.IndexOf (nameStart [i], j);
}
while (start >= 0 && j != 0 ? description [j++ - 1] == '{' : false);
....
return maxIndex == 1 ? "VALUE" : "VALUE" + (index + 1);
}
}Diagnostics worked on the line:
while (start >= 0 && j != 0 ? description [j++ - 1] == '{' : false)Since the variable j a few lines up is initialized to zero, the ternary operator will return the value false. Because of this condition, the body of the loop will be executed only once. It seems to me that this piece of code does not work at all as the programmer intended.
Warning N6
Analyzer warning: [CWE-571] Expression 'installedPackageVersions.Count != 1' is always true. NugetService.cs 1405
private void remove_nuget_cache_for_package(....)
{
if (!config.AllVersions && installedPackageVersions.Count > 1)
{
const string allVersionsChoice = "All versions";
if (installedPackageVersions.Count != 1)
{
choices.Add(allVersionsChoice);
}
....
}
....
}Here's the weird nested condition: installedPackageVersions.Count != 1, which will always be true. Often such a warning indicates a logical error in the code, and in other cases, simply an excessive check.
Warning N7
Analyzer warning: There are identical sub-expressions 'commandArguments.contains("-apikey")' to the left and to the right of the '||' operator. ArgumentsUtility.cs 42
public static bool arguments_contain_sensitive_information(string
commandArguments)
{
return commandArguments.contains("-install-arguments-sensitive")
|| commandArguments.contains("-package-parameters-sensitive")
|| commandArguments.contains("apikey ")
|| commandArguments.contains("config ")
|| commandArguments.contains("push ")
|| commandArguments.contains("-p ")
|| commandArguments.contains("-p=")
|| commandArguments.contains("-password")
|| commandArguments.contains("-cp ")
|| commandArguments.contains("-cp=")
|| commandArguments.contains("-certpassword")
|| commandArguments.contains("-k ")
|| commandArguments.contains("-k=")
|| commandArguments.contains("-key ")
|| commandArguments.contains("-key=")
|| commandArguments.contains("-apikey")
|| commandArguments.contains("-api-key")
|| commandArguments.contains("-apikey")
|| commandArguments.contains("-api-key");
}The programmer who wrote this piece of code copied and pasted the last two lines and forgot to edit them. Because of this, Chocolatey users have lost the ability to apply the parameter apikey a couple more ways. Similar to the parameters above, I can offer the following options:
commandArguments.contains("-apikey=");
commandArguments.contains("-api-key=");Copy-paste errors have a high chance of appearing sooner or later in any project with a large amount of source code, and one of the best means of dealing with them is static analysis.
PS And as always, this error tends to appear at the end of a multi-line condition :). See publication "".
Warning N8
Analyzer warning: [CWE-476] The 'installedPackage' object was used before it was verified against null. Check lines: 910, 917. NugetService.cs 910
public virtual ConcurrentDictionary<string, PackageResult> get_outdated(....)
{
....
var pinnedPackageResult = outdatedPackages.GetOrAdd(
packageName,
new PackageResult(installedPackage,
_fileSystem.combine_paths(
ApplicationParameters.PackagesLocation,
installedPackage.Id)));
....
if ( installedPackage != null
&& !string.IsNullOrWhiteSpace(installedPackage.Version.SpecialVersion)
&& !config.UpgradeCommand.ExcludePrerelease)
{
....
}
....
}Classic mistake: object first installedPackage used and then checked for null. This diagnostic tells us about one of two problems in the program: either installedPackage never equal null, which is doubtful, and then the check is redundant, or we can potentially get a serious error in the code - an attempt to access by a null reference.
Conclusion
So we have taken another small step - now using PVS-Studio has become even easier and more convenient. I also want to say that Chocolatey is a good package manager with a small number of bugs in the code, which could be even less with PVS-Studio.
We invite and try PVS-Studio. Regular use of a static analyzer will improve the quality and reliability of the code developed by your team and help prevent many .
PS
Before publication, we sent the article to the developers of Chocolatey, and they received it well. We did not find anything critical, but they, for example, liked the error we found related to the “api-key” key.
If you want to share this article with an English-speaking audience, then please use the link to the translation: Vladislav Stolyarov. .
Source: habr.com