Skip to content
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.

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

Crank up static analysis audit #2607

Merged
merged 56 commits into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
1989eb9
Make warnings errors for static analysis.
miniksa Aug 29, 2019
65dec36
C26446, Use .at instead of array indices
miniksa Aug 29, 2019
23897b1
[Complex] C26446, Use .at instead of array indices - Reword UTF8OutPi…
miniksa Aug 29, 2019
bd2d5dd
C26477, don't use 0 or NULL, use nullptr.
miniksa Aug 29, 2019
b33a598
C26496, mark const if it's never written after creation
miniksa Aug 29, 2019
c63289b
C26493, no C-style casts.
miniksa Aug 29, 2019
a381f6a
C26435, choose one of `virtual`, `override`, or `final`
miniksa Aug 29, 2019
8ea7401
C26472, no static_cast for arithmetic conversions. narrow or narrow_cast
miniksa Aug 29, 2019
50e2d0c
C26433, overrides should be explicit.
miniksa Aug 29, 2019
8579d89
C26451, promote before arithmetic if storing in larger result size (o…
miniksa Aug 29, 2019
8c3a629
C26481, don't use pointer arithemetic. use span.
miniksa Aug 29, 2019
4f1157c
C26447,C26440 - is noexcept but can throw or doesn't throw but not no…
miniksa Aug 29, 2019
30e8e7f
C26429, symbols not tested for nullness.
miniksa Sep 3, 2019
cdfbf8f
C26474, don't use static_cast when an implicit cast is acceptable.
miniksa Sep 3, 2019
230e7f4
C26466, disable dynamic_cast rule because we're not RTTI due to OS po…
miniksa Sep 3, 2019
7d4096b
C26485, refactor to avoid array-to-pointer decay.
miniksa Sep 3, 2019
81ab580
C26473, do not cast pointer back to the same type.
miniksa Sep 3, 2019
d5d7cf4
C26494, uninitalized local variables
miniksa Sep 3, 2019
bbdfdf9
C26462, const local variables that are unchanged.
miniksa Sep 3, 2019
b180406
C26445, wstring_view byref may indicate a lifetime issue
miniksa Sep 3, 2019
c956913
C26497, use constexpr for functions that could be evaluated at compil…
miniksa Sep 3, 2019
594dca9
C26429, mark gsl::not_null on places where we don't test for null (sh…
miniksa Sep 3, 2019
45e5993
C26430, not tested for nullness on all paths. I will just always chec…
miniksa Sep 3, 2019
9678dd8
C26414, don't use smart pointers for locals
miniksa Sep 3, 2019
dd49c3e
C26460, use const on params that are unchanged (and remove some unnec…
miniksa Sep 3, 2019
d8bc94f
forgot all return paths to _FillRectangle.
miniksa Sep 3, 2019
2d3f285
C26432, rule-of-five (if you define one of destruct/copy/move, then d…
miniksa Sep 3, 2019
3bbd8f4
C26443, overriding destructors shouldn't declare virtual nor override.
miniksa Sep 3, 2019
b78d917
C26434, do not hide base class methods. Overriding this one because i…
miniksa Sep 3, 2019
b87f8f9
C26426, global initializers calling non-constexpr. Suppress for defau…
miniksa Sep 3, 2019
072bbfd
C26426, global initializer calls non-constexpr. This needs further co…
miniksa Sep 3, 2019
5d60d69
C26426, global initializer calls non-constexpr. This is an easy move …
miniksa Sep 3, 2019
c7f0a34
C26490, don't reinterpret_cast. It looks like the buffer can easily b…
miniksa Sep 3, 2019
cd144e9
C26436, destructor definition required for class with virtual methods.
miniksa Sep 3, 2019
e14a59a
C26455, default constructor may not throw. Mark `noexcept`. (Trivial …
miniksa Sep 3, 2019
87f5852
Define actual constructor for CodepointWidthDetector as default isn't…
miniksa Sep 3, 2019
b2c093f
C26455, default constructor may not throw, mark as nothrow (another t…
miniksa Sep 3, 2019
3a0da64
C26490, no reinterpret_cast. Suppress on OutputCellIterator because f…
miniksa Sep 3, 2019
244fb72
C26490, no reinterpret_cast. Just use the actual struct and copy inst…
miniksa Sep 3, 2019
41f209f
C26440, default constructors should be noexcept.
miniksa Sep 3, 2019
93aa945
C26429, test for nullness or mark as not_null (and a few cascading wa…
miniksa Sep 3, 2019
ae25a32
C26497, you can mark this thing as constexpr.
miniksa Sep 3, 2019
01bd770
C26429, mark as not_null if not testing for nullness.
miniksa Sep 3, 2019
23b4a46
C26429, C26481, don't use pointer arithmetic, test for nullness. Also…
miniksa Sep 3, 2019
4204733
C26481, don't use pointer arithmetic. Convert to measuring string wit…
miniksa Sep 3, 2019
6735311
Suppress last two errors (C26455 default constructor throw in DxEngin…
miniksa Sep 3, 2019
7d9534b
constexprs have to go into the headers or other usages can't find the…
miniksa Sep 4, 2019
7c66e66
Fix redefinition of class name for constexpr method I moved from CPP …
miniksa Sep 4, 2019
3bff2a3
fix merge conflict with master
miniksa Sep 4, 2019
b7c1e05
code formatter, you're killing me.
miniksa Sep 4, 2019
d0c207b
fix remaining issues that appeared on merge.
miniksa Sep 4, 2019
96cc772
Add GH issue IDs to all the suppress/disables that I left behind as t…
miniksa Sep 5, 2019
689c21e
PR feedback.
miniksa Sep 5, 2019
fc81adf
use the array size for the read bounds. using extent on the newly-con…
miniksa Sep 5, 2019
d8ff47a
Some of the PR feedback.
miniksa Sep 6, 2019
18bacfe
A few PR comments. A constexpr here, a misleading comment there, and …
miniksa Sep 9, 2019
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
12 changes: 8 additions & 4 deletions src/StaticAnalysis.ruleset
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="Console Rules" Description="These rules enforce static analysis on console code." ToolsVersion="15.0">

<Include Path="cppcorecheckrules.ruleset" Action="Default" />
<Include Path="cppcorecheckrules.ruleset" Action="Error" />

<Rules AnalyzerId="Microsoft.Analyzers.NativeCodeAnalysis" RuleNamespace="Microsoft.Rules.Native">
<Rule Id="C6001" Action="Error" />
<Rule Id="C6011" Action="Error" />
<Rules AnalyzerId="Microsoft.Analyzers.NativeCodeAnalysis" RuleNamespace="Microsoft.Rules.Native">
<Rule Id="C6001" Action="Error" />
<Rule Id="C6011" Action="Error" />
<!-- We can't do dynamic cast because RTTI is off. -->
<!-- RTTI is off because Windows OS policies believe RTTI has too much binary size impact for the value and is less portable than RTTI-off modules. -->
<Rule Id="C26466" Action="None" />
</Rules>


</RuleSet>
10 changes: 5 additions & 5 deletions src/buffer/out/AttrRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void ATTR_ROW::Resize(const size_t newWidth)
{
// Get the attribute that covers the final column of old width.
const auto runPos = FindAttrIndex(_cchRowWidth - 1, nullptr);
auto& run = _list[runPos];
auto& run = _list.at(runPos);

// Extend its length by the additional columns we're adding.
run.SetLength(run.GetLength() + newWidth - _cchRowWidth);
Expand All @@ -60,7 +60,7 @@ void ATTR_ROW::Resize(const size_t newWidth)
// Get the attribute that covers the final column of the new width
size_t CountOfAttr = 0;
const auto runPos = FindAttrIndex(newWidth - 1, &CountOfAttr);
auto& run = _list[runPos];
auto& run = _list.at(runPos);

// CountOfAttr was given to us as "how many columns left from this point forward are covered by the returned run"
// So if the original run was B5 covering a 5 size OldWidth and we have a NewWidth of 3
Expand Down Expand Up @@ -108,7 +108,7 @@ TextAttribute ATTR_ROW::GetAttrByColumn(const size_t column,
{
THROW_HR_IF(E_INVALIDARG, column >= _cchRowWidth);
const auto runPos = FindAttrIndex(column, pApplies);
return _list[runPos].GetAttributes();
return _list.at(runPos).GetAttributes();
}

// Routine Description:
Expand Down Expand Up @@ -290,10 +290,10 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// two elements in our internal list.
else if (_list.size() == 2 && newAttrs.at(0).GetLength() == 1)
{
auto left = _list.begin();
const auto left = _list.begin();
if (iStart == left->GetLength() && NewAttr == left->GetAttributes())
{
auto right = left + 1;
const auto right = left + 1;
left->IncrementLength();
right->DecrementLength();

Expand Down
8 changes: 4 additions & 4 deletions src/buffer/out/AttrRowIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@
#include "AttrRowIterator.hpp"
#include "AttrRow.hpp"

AttrRowIterator AttrRowIterator::CreateEndIterator(const ATTR_ROW* const attrRow)
AttrRowIterator AttrRowIterator::CreateEndIterator(const ATTR_ROW* const attrRow) noexcept
{
AttrRowIterator it{ attrRow };
it._setToEnd();
return it;
}

AttrRowIterator::AttrRowIterator(const ATTR_ROW* const attrRow) :
AttrRowIterator::AttrRowIterator(const ATTR_ROW* const attrRow) noexcept :
_pAttrRow{ attrRow },
_run{ attrRow->_list.cbegin() },
_currentAttributeIndex{ 0 }
{
}

AttrRowIterator::operator bool() const noexcept
AttrRowIterator::operator bool() const
{
return _run < _pAttrRow->_list.cend();
}
Expand Down Expand Up @@ -139,7 +139,7 @@ void AttrRowIterator::_decrement(size_t count)

// Routine Description:
// - sets fields on the iterator to describe the end() state of the ATTR_ROW
void AttrRowIterator::_setToEnd()
void AttrRowIterator::_setToEnd() noexcept
{
_run = _pAttrRow->_list.cend();
_currentAttributeIndex = 0;
Expand Down
8 changes: 4 additions & 4 deletions src/buffer/out/AttrRowIterator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ class AttrRowIterator final
using pointer = TextAttribute*;
using reference = TextAttribute&;

static AttrRowIterator CreateEndIterator(const ATTR_ROW* const attrRow);
static AttrRowIterator CreateEndIterator(const ATTR_ROW* const attrRow) noexcept;

AttrRowIterator(const ATTR_ROW* const attrRow);
AttrRowIterator(const ATTR_ROW* const attrRow) noexcept;

operator bool() const noexcept;
operator bool() const;

bool operator==(const AttrRowIterator& it) const;
bool operator!=(const AttrRowIterator& it) const;
Expand All @@ -57,5 +57,5 @@ class AttrRowIterator final

void _increment(size_t count);
void _decrement(size_t count);
void _setToEnd();
void _setToEnd() noexcept;
};
39 changes: 8 additions & 31 deletions src/buffer/out/CharRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ size_t CharRow::size() const noexcept
// - sRowWidth - The width of the row.
// Return Value:
// - <none>
void CharRow::Reset()
void CharRow::Reset() noexcept
{
for (auto& cell : _data)
{
Expand Down Expand Up @@ -209,7 +209,7 @@ const DbcsAttribute& CharRow::DbcsAttrAt(const size_t column) const
// Note: will throw exception if column is out of bounds
DbcsAttribute& CharRow::DbcsAttrAt(const size_t column)
{
return const_cast<DbcsAttribute&>(static_cast<const CharRow* const>(this)->DbcsAttrAt(column));
return _data.at(column).DbcsAttr();
}

// Routine Description:
Expand Down Expand Up @@ -250,54 +250,31 @@ CharRow::reference CharRow::GlyphAt(const size_t column)
return { *this, column };
}

// Routine Description:
// - returns string containing text data exactly how it's stored internally, including doubling of
// leading/trailing cells.
// Arguments:
// - none
// Return Value:
// - text stored in char row
// - Note: will throw exception if out of memory
std::wstring CharRow::GetTextRaw() const
{
std::wstring wstr;
wstr.reserve(_data.size());
for (size_t i = 0; i < _data.size(); ++i)
{
auto glyph = GlyphAt(i);
for (auto it = glyph.begin(); it != glyph.end(); ++it)
{
wstr.push_back(*it);
}
}
return wstr;
}

std::wstring CharRow::GetText() const
{
std::wstring wstr;
wstr.reserve(_data.size());

for (size_t i = 0; i < _data.size(); ++i)
{
auto glyph = GlyphAt(i);
const auto glyph = GlyphAt(i);
if (!DbcsAttrAt(i).IsTrailing())
{
for (auto it = glyph.begin(); it != glyph.end(); ++it)
for (const auto wch : glyph)
{
wstr.push_back(*it);
wstr.push_back(wch);
}
}
}
return wstr;
}

UnicodeStorage& CharRow::GetUnicodeStorage()
UnicodeStorage& CharRow::GetUnicodeStorage() noexcept
{
return _pParent->GetUnicodeStorage();
}

const UnicodeStorage& CharRow::GetUnicodeStorage() const
const UnicodeStorage& CharRow::GetUnicodeStorage() const noexcept
{
return _pParent->GetUnicodeStorage();
}
Expand All @@ -308,7 +285,7 @@ const UnicodeStorage& CharRow::GetUnicodeStorage() const
// - column - the column to generate the key for
// Return Value:
// - the COORD key for data access from UnicodeStorage for the column
COORD CharRow::GetStorageKey(const size_t column) const
COORD CharRow::GetStorageKey(const size_t column) const noexcept
{
return { gsl::narrow<SHORT>(column), _pParent->GetId() };
}
Expand Down
11 changes: 4 additions & 7 deletions src/buffer/out/CharRow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class CharRow final
void SetDoubleBytePadded(const bool doubleBytePadded) noexcept;
bool WasDoubleBytePadded() const noexcept;
size_t size() const noexcept;
void Reset();
void Reset() noexcept;
[[nodiscard]] HRESULT Resize(const size_t newSize) noexcept;
size_t MeasureLeft() const;
size_t MeasureRight() const noexcept;
Expand All @@ -64,9 +64,6 @@ class CharRow final
void ClearGlyph(const size_t column);
std::wstring GetText() const;

// other functions implemented at the template class level
std::wstring GetTextRaw() const;

// working with glyphs
const reference GlyphAt(const size_t column) const;
reference GlyphAt(const size_t column);
Expand All @@ -78,9 +75,9 @@ class CharRow final
iterator end() noexcept;
const_iterator cend() const noexcept;

UnicodeStorage& GetUnicodeStorage();
const UnicodeStorage& GetUnicodeStorage() const;
COORD GetStorageKey(const size_t column) const;
UnicodeStorage& GetUnicodeStorage() noexcept;
const UnicodeStorage& GetUnicodeStorage() const noexcept;
COORD GetStorageKey(const size_t column) const noexcept;

void UpdateParent(ROW* const pParent) noexcept;

Expand Down
6 changes: 3 additions & 3 deletions src/buffer/out/CharRowCell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,21 @@
// default glyph value, used for reseting the character data portion of a cell
static constexpr wchar_t DefaultValue = UNICODE_SPACE;

CharRowCell::CharRowCell() :
CharRowCell::CharRowCell() noexcept :
_wch{ DefaultValue },
_attr{}
{
}

CharRowCell::CharRowCell(const wchar_t wch, const DbcsAttribute attr) :
CharRowCell::CharRowCell(const wchar_t wch, const DbcsAttribute attr) noexcept :
_wch{ wch },
_attr{ attr }
{
}

// Routine Description:
// - "erases" the glyph. really sets it back to the default "empty" value
void CharRowCell::EraseChars()
void CharRowCell::EraseChars() noexcept
{
if (_attr.IsGlyphStored())
{
Expand Down
6 changes: 3 additions & 3 deletions src/buffer/out/CharRowCell.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ Author(s):
class CharRowCell final
{
public:
CharRowCell();
CharRowCell(const wchar_t wch, const DbcsAttribute attr);
CharRowCell() noexcept;
CharRowCell(const wchar_t wch, const DbcsAttribute attr) noexcept;

void EraseChars();
void EraseChars() noexcept;
void Reset() noexcept;

bool IsSpace() const noexcept;
Expand Down
4 changes: 4 additions & 0 deletions src/buffer/out/CharRowCellReference.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ CharRowCellReference::const_iterator CharRowCellReference::begin() const
// - get read-only iterator to the end of the glyph data
// Return Value:
// - end iterator of the glyph data
#pragma warning(push)
#pragma warning(disable : 26481)
// TODO: eliminate using pointers raw as begin/end markers in this class
CharRowCellReference::const_iterator CharRowCellReference::end() const
{
if (_cellData().DbcsAttr().IsGlyphStored())
Expand All @@ -103,6 +106,7 @@ CharRowCellReference::const_iterator CharRowCellReference::end() const
return &_cellData().Char() + 1;
}
}
#pragma warning(pop)

bool operator==(const CharRowCellReference& ref, const std::vector<wchar_t>& glyph)
{
Expand Down
2 changes: 1 addition & 1 deletion src/buffer/out/CharRowCellReference.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class CharRowCellReference final
public:
using const_iterator = const wchar_t*;

CharRowCellReference(CharRow& parent, const size_t index) :
CharRowCellReference(CharRow& parent, const size_t index) noexcept :
_parent{ parent },
_index{ index }
{
Expand Down
2 changes: 1 addition & 1 deletion src/buffer/out/DbcsAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class DbcsAttribute final
return _glyphStored;
}

void SetGlyphStored(const bool stored)
void SetGlyphStored(const bool stored) noexcept
{
_glyphStored = stored;
}
Expand Down
4 changes: 2 additions & 2 deletions src/buffer/out/OutputCell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

static constexpr TextAttribute InvalidTextAttribute{ INVALID_COLOR, INVALID_COLOR };

OutputCell::OutputCell() :
OutputCell::OutputCell() noexcept :
_text{},
_dbcsAttribute{},
_textAttribute{ InvalidTextAttribute },
Expand Down Expand Up @@ -112,6 +112,6 @@ void OutputCell::_setFromOutputCellView(const OutputCellView& cell)
_textAttribute = cell.TextAttr();
_behavior = cell.TextAttrBehavior();

const auto& view = cell.Chars();
const auto view = cell.Chars();
_text = view;
miniksa marked this conversation as resolved.
Show resolved Hide resolved
}
4 changes: 2 additions & 2 deletions src/buffer/out/OutputCell.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Module Name:

class InvalidCharInfoConversionException : public std::exception
{
const char* what() const noexcept
const char* what() const noexcept override
{
return "Cannot convert to CHAR_INFO without explicit TextAttribute";
}
Expand All @@ -34,7 +34,7 @@ class InvalidCharInfoConversionException : public std::exception
class OutputCell final
{
public:
OutputCell();
OutputCell() noexcept;

OutputCell(const std::wstring_view charData,
const DbcsAttribute dbcsAttribute,
Expand Down
Loading