Skip to content

Enable code analysis and address initial set of issues. #58304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 10, 2024
Merged

Enable code analysis and address initial set of issues. #58304

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2747640
Enable code analysis and fix NULL usage.
adityamandaleeka Oct 4, 2024
0a62055
Fix several C6xxx memory warnings in ANCM.
adityamandaleeka Oct 6, 2024
321c2aa
Enable CA for CommonLib and address additional errors.
adityamandaleeka Oct 8, 2024
0959589
Fix C26474 (use implicit conversion).
adityamandaleeka Oct 8, 2024
d35b770
Fix unnanotated fallthroughs - C26819
adityamandaleeka Oct 8, 2024
d0e7f8c
Fix SAL related CA warnings.
adityamandaleeka Oct 8, 2024
7aa9be1
Fix constexpr warnings and constant std::move warnings C26478 C26497.
adityamandaleeka Oct 8, 2024
c1752f6
Minor improvements.
adityamandaleeka Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt
auto shadowCopyBaseDirectory = std::filesystem::directory_entry(shadowCopyPath);
if (!shadowCopyBaseDirectory.exists())
{
CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), NULL);
CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), nullptr);
}

for (auto& entry : std::filesystem::directory_iterator(shadowCopyPath))
Expand Down
3 changes: 2 additions & 1 deletion src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="..\..\build\Build.Lib.Settings" />
<PropertyGroup Label="Globals">
<ProjectGuid>{55494E58-E061-4C4C-A0A8-837008E72F85}</ProjectGuid>
<RootNamespace>NewCommon</RootNamespace>
<RunCodeAnalysis>false</RunCodeAnalysis>
</PropertyGroup>
<Import Project="..\..\build\Build.Lib.Settings" />
<ItemDefinitionGroup>
<ClCompile>
<AdditionalIncludeDirectories>..\iislib;$(LibNetHostPath)</AdditionalIncludeDirectories>
Expand Down
18 changes: 9 additions & 9 deletions src/Servers/IIS/AspNetCoreModuleV2/CommonLib/sttimer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class STTIMER
public:

STTIMER()
: _pTimer( NULL )
: _pTimer( nullptr )
{
fInCanel = FALSE;
}
Expand All @@ -25,7 +25,7 @@ class STTIMER
{
CancelTimer();
CloseThreadpoolTimer( _pTimer );
_pTimer = NULL;
_pTimer = nullptr;
}
}

Expand All @@ -39,7 +39,7 @@ class STTIMER
{
_pTimer = CreateThreadpoolTimer( pfnCallback,
pContext,
NULL );
nullptr );

if ( !_pTimer )
{
Expand Down Expand Up @@ -74,9 +74,9 @@ class STTIMER
// re-enabled by setting non-zero initial wait or
// period values.
//
if (_pTimer != NULL)
if (_pTimer != nullptr)
{
SetThreadpoolTimer(_pTimer, NULL, 0, 0);
SetThreadpoolTimer(_pTimer, nullptr, 0, 0);
}

return;
Expand Down Expand Up @@ -106,7 +106,7 @@ class STTIMER
// Wait until any callbacks queued prior to disabling
// have completed.
//
if (_pTimer != NULL)
if (_pTimer != nullptr)
{
WaitForThreadpoolTimerCallbacks(_pTimer, TRUE);
}
Expand All @@ -124,21 +124,21 @@ class STTIMER
)
{
STRU* pstruLogFilePath = (STRU*)Context;
HANDLE hStdoutHandle = NULL;
HANDLE hStdoutHandle = nullptr;
SECURITY_ATTRIBUTES saAttr = { 0 };
HRESULT hr = S_OK;

saAttr.nLength = sizeof(SECURITY_ATTRIBUTES);
saAttr.bInheritHandle = TRUE;
saAttr.lpSecurityDescriptor = NULL;
saAttr.lpSecurityDescriptor = nullptr;

hStdoutHandle = CreateFileW(pstruLogFilePath->QueryStr(),
FILE_READ_DATA,
FILE_SHARE_WRITE,
&saAttr,
OPEN_ALWAYS,
FILE_ATTRIBUTE_NORMAL,
NULL);
nullptr);
if (hStdoutHandle == INVALID_HANDLE_VALUE)
{
hr = HRESULT_FROM_WIN32(GetLastError());
Expand Down
2 changes: 1 addition & 1 deletion src/Servers/IIS/AspNetCoreModuleV2/CommonLibTests/CommonLibTests.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<ProjectGuid>{1eac8125-1765-4e2d-8cbe-56dc98a1c8c1}</ProjectGuid>
<Keyword>Win32Proj</Keyword>
<ConfigurationType>Application</ConfigurationType>
<PlatformToolset>$(PlatformToolsetVersion)</PlatformToolset>
<PlatformToolset>v$(PlatformToolsetVersion)</PlatformToolset>
<CharacterSet>Unicode</CharacterSet>
<IsTestProject>true</IsTestProject>
<DisableArcadeTestFramework>true</DisableArcadeTestFramework>
Expand Down
130 changes: 70 additions & 60 deletions src/Servers/IIS/AspNetCoreModuleV2/DefaultRules.ruleset
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="DefaultRules" ToolsVersion="15.0">
<RuleSet Name="DefaultRules" ToolsVersion="17.0">
<Include Path="allrules.ruleset" Action="Default" />
<Rules AnalyzerId="Microsoft.Analyzers.NativeCodeAnalysis" RuleNamespace="Microsoft.Rules.Native">
<Rule Id="C26100" Action="Error" />
Expand All @@ -12,88 +12,97 @@
<Rule Id="C26116" Action="Error" />
<Rule Id="C26117" Action="Error" />
<Rule Id="C26130" Action="Error" />
<Rule Id="C26135" Action="Error" />
<Rule Id="C26135" Action="None" /> <!-- Missing annotation <annotation> at function '<func>' -->
<Rule Id="C26140" Action="Error" />
<Rule Id="C26160" Action="Error" />
<Rule Id="C26160" Action="None" /> <!-- Caller possibly failing to hold lock '<lock>' before calling function '<func>'. -->
<Rule Id="C26165" Action="Error" />
<Rule Id="C26166" Action="Error" />
<Rule Id="C26167" Action="Error" />
<Rule Id="C26400" Action="Error" />
<Rule Id="C26401" Action="Error" />
<Rule Id="C26400" Action="None" /> <!-- Do not assign the result of an allocation or a function call with an owner<T> return value to a raw pointer -->
<Rule Id="C26401" Action="None" /> <!-- Do not delete a raw pointer that is not an owner<T> -->
<Rule Id="C26402" Action="Error" />
<Rule Id="C26403" Action="Error" />
<Rule Id="C26404" Action="Error" />
<Rule Id="C26405" Action="Error" />
<Rule Id="C26406" Action="Error" />
<Rule Id="C26407" Action="Error" />
<Rule Id="C26408" Action="Error" />
<Rule Id="C26409" Action="Error" />
<Rule Id="C26409" Action="None" /> <!-- Avoid calling new and delete explicitly, use std::make_unique<T> instead -->
<Rule Id="C26410" Action="Error" />
<Rule Id="C26411" Action="Error" />
<Rule Id="C26414" Action="Error" />
<Rule Id="C26415" Action="Error" />
<Rule Id="C26414" Action="None" /> <!-- Move, copy, reassign or reset a local smart pointer. -->
<Rule Id="C26415" Action="None" /> <!-- Smart pointer parameter is used only to access contained pointer. Use T* or T& instead. -->
<Rule Id="C26416" Action="Error" />
<Rule Id="C26417" Action="Error" />
<Rule Id="C26418" Action="Error" />
<Rule Id="C26418" Action="None" /> <!-- Shared pointer parameter is not copied or moved. Use T* or T& instead. -->
<Rule Id="C26426" Action="Error" />
<Rule Id="C26427" Action="Error" />
<Rule Id="C26429" Action="None" />
<Rule Id="C26430" Action="Error" />
<Rule Id="C26429" Action="None" /> <!-- Symbol is never tested for nullness, it can be marked as gsl::not_null. -->
<Rule Id="C26430" Action="None" /> <!-- Symbol '<var>' is not tested for nullness on all paths -->
<Rule Id="C26431" Action="Error" />
<Rule Id="C26432" Action="None" />
<Rule Id="C26433" Action="Error" />
<Rule Id="C26434" Action="Error" />
<Rule Id="C26432" Action="None" /> <!-- If you define or delete any default operation in the type 'type-name', define or delete them all -->
<Rule Id="C26433" Action="None" /> <!-- Function should be marked with override -->
<Rule Id="C26434" Action="None" /> <!-- Function 'derived::function' hides a non-virtual function 'base::function' -->
<Rule Id="C26435" Action="Error" />
<Rule Id="C26436" Action="Error" />
<Rule Id="C26436" Action="None" /> <!-- The type 'symbol' with a virtual function needs either public virtual or protected non-virtual destructor -->
<Rule Id="C26437" Action="Error" />
<Rule Id="C26438" Action="Error" />
<Rule Id="C26438" Action="None" /> <!-- Avoid goto -->
<Rule Id="C26439" Action="Error" />
<Rule Id="C26440" Action="Error" />
<Rule Id="C26440" Action="None" /> <!-- Function can be declared 'noexcept'. -->
<Rule Id="C26441" Action="Error" />
<Rule Id="C26443" Action="Error" />
<Rule Id="C26444" Action="Error" />
<Rule Id="C26445" Action="Error" />
<Rule Id="C26446" Action="None" />
<Rule Id="C26447" Action="Error" />
<Rule Id="C26448" Action="None" />
<Rule Id="C26446" Action="None" /> <!-- Prefer to use gsl::at() instead of unchecked subscript operator -->
<Rule Id="C26447" Action="None" /> <!-- The function is declared noexcept but calls function function_name that may throw exceptions -->
<Rule Id="C26448" Action="None" /> <!-- Consider using gsl::finally if final action is intended -->
<Rule Id="C26449" Action="Error" />
<Rule Id="C26450" Action="Error" />
<Rule Id="C26451" Action="Error" />
<Rule Id="C26451" Action="None" /> <!-- Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow -->
<Rule Id="C26452" Action="Error" />
<Rule Id="C26453" Action="Error" />
<Rule Id="C26454" Action="Error" />
<Rule Id="C26455" Action="None" /> <!-- Default constructor should not throw. Declare it 'noexcept' -->
<Rule Id="C26457" Action="None" /> <!-- (void) should not be used to ignore return values, use 'std::ignore =' instead -->
<Rule Id="C26459" Action="Error" />
<Rule Id="C26460" Action="Error" />
<Rule Id="C26461" Action="Error" />
<Rule Id="C26462" Action="Error" />
<Rule Id="C26460" Action="None" /> <!-- The reference argument 'argument' for function 'function' can be marked as const -->
<Rule Id="C26461" Action="None" /> <!-- The pointer argument 'argument' for function 'function' can be marked as a pointer to const -->
<Rule Id="C26462" Action="None" /> <!-- The value pointed to by '<var>' is assigned only once, mark it as a pointer to const -->
<Rule Id="C26463" Action="Error" />
<Rule Id="C26464" Action="Error" />
<Rule Id="C26465" Action="Error" />
<Rule Id="C26466" Action="Error" />
<Rule Id="C26471" Action="Error" />
<Rule Id="C26472" Action="None" />
<Rule Id="C26473" Action="Error" />
<Rule Id="C26474" Action="Error" />
<Rule Id="C26467" Action="None" /> <!-- Converting from floating point to unsigned integral types results in non-portable code if the double/float has a negative value. -->
<Rule Id="C26471" Action="None" /> <!-- Don't use reinterpret_cast. A cast from void* can use static_cast -->
<Rule Id="C26472" Action="None" /> <!-- Don't use a static_cast for arithmetic conversions. Use brace initialization, gsl::narrow_cast, or gsl::narrow. -->
<Rule Id="C26473" Action="None" /> <!-- Don't cast between pointer types where the source type and the target type are the same -->
<Rule Id="C26474" Action="None" /> <!-- Use implicit conversion -->
<Rule Id="C26475" Action="Error" />
<Rule Id="C26476" Action="Error" />
<Rule Id="C26476" Action="None" /> <!-- Expression/symbol 'name' uses a naked union 'union' with multiple type pointers: Use variant instead -->
<Rule Id="C26477" Action="Error" />
<Rule Id="C26481" Action="Error" />
<Rule Id="C26482" Action="Error" />
<Rule Id="C26478" Action="None" /> <!-- Don't use std::move on constant variables. -->
<Rule Id="C26481" Action="None" /> <!-- Don't use pointer arithmetic. Use span instead -->
<Rule Id="C26482" Action="None" /> <!-- Only index into arrays using constant expressions. -->
<Rule Id="C26483" Action="Error" />
<Rule Id="C26485" Action="Error" />
<Rule Id="C26486" Action="None" />
<Rule Id="C26487" Action="Error" />
<Rule Id="C26489" Action="None" />
<Rule Id="C26490" Action="Error" />
<Rule Id="C26485" Action="None" /> <!-- Expression 'array-name': No array to pointer decay -->
<Rule Id="C26486" Action="None" /> <!-- Don't pass a pointer that may be invalid (dangling) as a parameter to a function. -->
<Rule Id="C26487" Action="None" /> <!-- Don't allow a function to return an invalid pointer, either through a formal return statement or through output parameters. -->
<Rule Id="C26489" Action="None" /> <!-- Don't dereference a pointer that may be invalid. -->
<Rule Id="C26490" Action="None" /> <!-- Don't use reinterpret_cast. -->
<Rule Id="C26491" Action="Error" />
<Rule Id="C26492" Action="Error" />
<Rule Id="C26493" Action="Error" />
<Rule Id="C26494" Action="Error" />
<Rule Id="C26495" Action="Error" />
<Rule Id="C26496" Action="Error" />
<Rule Id="C26497" Action="Error" />
<Rule Id="C26492" Action="None" /> <!-- Don't use const_cast to cast away const. -->
<Rule Id="C26493" Action="None" /> <!-- Don't use C-style casts. -->
<Rule Id="C26494" Action="None" /> <!-- Variable 'variable' is uninitialized. Always initialize an object. -->
<Rule Id="C26495" Action="None" /> <!-- Variable 'variable' is uninitialized. Always initialize a member variable -->
<Rule Id="C26496" Action="None" /> <!-- The variable 'variable' is assigned only once, mark it as const. -->
<Rule Id="C26497" Action="None" /> <!-- Attempt to make this constexpr -->
<Rule Id="C26498" Action="Error" />
<Rule Id="C28020" Action="Error" />
<Rule Id="C26814" Action="None" /> <!-- The const variable 'variable' can be computed at compile time. Consider using constexpr -->
<Rule Id="C26818" Action="None" /> <!-- Switch statement does not cover all cases. Consider adding a 'default' label -->
<Rule Id="C26819" Action="None" /> <!-- Unannotated fallthrough -->
<Rule Id="C26826" Action="None" /> <!-- Don't use C-style variable arguments -->
<Rule Id="C26859" Action="None" /> <!-- Empty optional '<var>' is unwrapped, will throw exception. -->
<Rule Id="C28020" Action="None" /> <!-- The expression 'expr' is not true at this call -->
<Rule Id="C28021" Action="Error" />
<Rule Id="C28022" Action="Error" />
<Rule Id="C28023" Action="Error" />
Expand Down Expand Up @@ -142,7 +151,7 @@
<Rule Id="C28156" Action="Error" />
<Rule Id="C28157" Action="Error" />
<Rule Id="C28158" Action="Error" />
<Rule Id="C28159" Action="Error" />
<Rule Id="C28159" Action="None" /> <!-- Consider using *function_name_1* instead of *function_name_2*. -->
<Rule Id="C28160" Action="Error" />
<Rule Id="C28161" Action="Error" />
<Rule Id="C28162" Action="Error" />
Expand Down Expand Up @@ -213,9 +222,9 @@
<Rule Id="C28245" Action="Error" />
<Rule Id="C28246" Action="Error" />
<Rule Id="C28250" Action="Error" />
<Rule Id="C28251" Action="Error" />
<Rule Id="C28252" Action="Error" />
<Rule Id="C28253" Action="Error" />
<Rule Id="C28251" Action="None" /> <!-- Inconsistent annotation for function: this instance has an error -->
<Rule Id="C28252" Action="None" /> <!-- Inconsistent annotation for '<func>': <param> has '<annotation>' on the prior instance. -->
<Rule Id="C28253" Action="None" /> <!-- Inconsistent annotation for '<func>': <param> has '<annotations>' on this instance -->
<Rule Id="C28254" Action="Error" />
<Rule Id="C28260" Action="Error" />
<Rule Id="C28262" Action="Error" />
Expand All @@ -239,7 +248,7 @@
<Rule Id="C28290" Action="Error" />
<Rule Id="C28291" Action="Error" />
<Rule Id="C28300" Action="Error" />
<Rule Id="C28301" Action="Error" />
<Rule Id="C28301" Action="None" /> <!-- No annotations for first declaration -->
<Rule Id="C28302" Action="Error" />
<Rule Id="C28303" Action="Error" />
<Rule Id="C28304" Action="Error" />
Expand Down Expand Up @@ -282,19 +291,19 @@
<Rule Id="C28736" Action="Error" />
<Rule Id="C28750" Action="Error" />
<Rule Id="C28751" Action="Error" />
<Rule Id="C6001" Action="Error" />
<Rule Id="C6011" Action="Error" />
<Rule Id="C6001" Action="None" /> <!-- uninitialized memory -->
<Rule Id="C6011" Action="None" /> <!-- Possible null deref -->
<Rule Id="C6014" Action="Error" />
<Rule Id="C6029" Action="Error" />
<Rule Id="C6031" Action="Error" />
<Rule Id="C6031" Action="None" /> <!-- Return value ignored: '<func>'. -->
<Rule Id="C6053" Action="Error" />
<Rule Id="C6054" Action="Error" />
<Rule Id="C6054" Action="None" /> <!-- String '<var>' might not be zero-terminated. -->
<Rule Id="C6059" Action="Error" />
<Rule Id="C6063" Action="Error" />
<Rule Id="C6064" Action="Error" />
<Rule Id="C6066" Action="Error" />
<Rule Id="C6067" Action="Error" />
<Rule Id="C6101" Action="Error" />
<Rule Id="C6101" Action="None" /> <!-- Returning uninitialized memory -->
<Rule Id="C6200" Action="Error" />
<Rule Id="C6201" Action="Error" />
<Rule Id="C6211" Action="Error" />
Expand All @@ -319,7 +328,7 @@
<Rule Id="C6248" Action="Error" />
<Rule Id="C6250" Action="Error" />
<Rule Id="C6255" Action="Error" />
<Rule Id="C6258" Action="Error" />
<Rule Id="C6258" Action="None" /> <!-- Using TerminateThread does not allow proper thread clean up. -->
<Rule Id="C6259" Action="Error" />
<Rule Id="C6260" Action="Error" />
<Rule Id="C6262" Action="Error" />
Expand All @@ -338,7 +347,7 @@
<Rule Id="C6280" Action="Error" />
<Rule Id="C6281" Action="Error" />
<Rule Id="C6282" Action="Error" />
<Rule Id="C6283" Action="Error" />
<Rule Id="C6283" Action="None" /> <!-- array new with scalar delete -->
<Rule Id="C6284" Action="Error" />
<Rule Id="C6285" Action="Error" />
<Rule Id="C6286" Action="Error" />
Expand Down Expand Up @@ -383,14 +392,14 @@
<Rule Id="C6334" Action="Error" />
<Rule Id="C6335" Action="Error" />
<Rule Id="C6336" Action="Error" />
<Rule Id="C6340" Action="Error" />
<Rule Id="C6340" Action="None" /> <!-- sign mismatch -->
<Rule Id="C6381" Action="Error" />
<Rule Id="C6383" Action="Error" />
<Rule Id="C6384" Action="Error" />
<Rule Id="C6385" Action="Error" />
<Rule Id="C6386" Action="Error" />
<Rule Id="C6387" Action="Error" />
<Rule Id="C6388" Action="Error" />
<Rule Id="C6385" Action="None" /> <!-- Reading invalid data -->
<Rule Id="C6386" Action="None" /> <!-- Overrun -->
<Rule Id="C6387" Action="None" /> <!-- '<expr>' could be '<val>': this does not adhere to the specification for the function '<func>' -->
<Rule Id="C6388" Action="None" /> <!-- '<var>' might not be '<val>': this does not adhere to the specification for the function '<func>' -->
<Rule Id="C6400" Action="Error" />
<Rule Id="C6401" Action="Error" />
<Rule Id="C6411" Action="Error" />
Expand Down Expand Up @@ -418,6 +427,7 @@
<Rule Id="C6540" Action="Error" />
<Rule Id="C6551" Action="Error" />
<Rule Id="C6552" Action="Error" />
<Rule Id="C6553" Action="None" /> <!-- Annotation does not apply to a value type -->
<Rule Id="C6701" Action="Error" />
<Rule Id="C6702" Action="Error" />
<Rule Id="C6703" Action="Error" />
Expand Down
Loading