Initialize all members of Terminal #14345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
Area-Input
zadjii-msft
approved these changes
Nov 7, 2022
| @@ -37,24 +37,7 @@ static std::wstring _KeyEventsToText(std::deque<std::unique_ptr<IInputEvent>>& i | |||
| } | |||
|
|
|||
| #pragma warning(suppress : 26455) // default constructor is throwing, too much effort to rearrange at this time. | |||
| Terminal::Terminal() : | |||
| _mutableViewport{ Viewport::Empty() }, | |||
There was a problem hiding this comment.
This one didn't get copied over to the header. Should we initialize it there too?
Comment on lines
364
to
365
| std::unique_ptr<TextBuffer> _mainBuffer; | ||
| std::unique_ptr<TextBuffer> _altBuffer; |
There was a problem hiding this comment.
Suggested change
| std::unique_ptr<TextBuffer> _mainBuffer; | |
| std::unique_ptr<TextBuffer> _altBuffer; | |
| std::unique_ptr<TextBuffer> _mainBuffer = nullptr; | |
| std::unique_ptr<TextBuffer> _altBuffer = nullptr; |
no benefit to initializing these too? At least for consistency.
| @@ -356,27 +352,27 @@ class Microsoft::Terminal::Core::Terminal final : | |||
| til::point pivot; | |||
| }; | |||
| std::optional<SelectionAnchors> _selection; | |||
There was a problem hiding this comment.
Suggested change
| std::optional<SelectionAnchors> _selection; | |
| std::optional<SelectionAnchors> _selection = std::nullopt; |
this one also didn't get carried over from the cpp.
There was a problem hiding this comment.
optionals are technically .. ah you probably talked about this
carlos-zamora
approved these changes
Nov 7, 2022
lhecker
added
the
AutoMerge
|
Hello @lhecker! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
DHowett
pushed a commit
that referenced
this pull request
Dec 1, 2022
The following members were not initialized during construction: * `CursorType _defaultCursorShape` * `bool _suppressApplicationTitle` * `bool _bracketedPasteMode` * `size_t _hyperlinkPatternId` * `SelectionExpansion _multiClickSelectionMode` * `til::CoordType _scrollbackLines` Unlike gcc and clang, MSVC is fairly tame when it comes to removing code tainted by undefined behavior, so the most likely affect this had is that we were reading uninitialized memory. Related to #14129. (cherry picked from commit a798a60) Service-Card-Id: 86826740 Service-Version: 1.15
DHowett
pushed a commit
that referenced
this pull request
Dec 1, 2022
The following members were not initialized during construction: * `CursorType _defaultCursorShape` * `bool _suppressApplicationTitle` * `bool _bracketedPasteMode` * `size_t _hyperlinkPatternId` * `SelectionExpansion _multiClickSelectionMode` * `til::CoordType _scrollbackLines` Unlike gcc and clang, MSVC is fairly tame when it comes to removing code tainted by undefined behavior, so the most likely affect this had is that we were reading uninitialized memory. Related to #14129. (cherry picked from commit a798a60) Service-Card-Id: 86603221 Service-Version: 1.16
|
🎉 Handy links: |
|
🎉 Handy links: |
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following members were not initialized during construction:
CursorType _defaultCursorShapebool _suppressApplicationTitlebool _bracketedPasteModesize_t _hyperlinkPatternIdSelectionExpansion _multiClickSelectionModetil::CoordType _scrollbackLinesUnlike gcc and clang, MSVC is fairly tame when it comes to removing code
tainted by undefined behavior, so the most likely affect this had is that
we were reading uninitialized memory.
Related to #14129.