From f9e71445d9c58552d0af1461b7c4990e6be6d3a4 Mon Sep 17 00:00:00 2001
From: James Holderness <j4_james@hotmail.com>
Date: Sat, 7 Sep 2019 02:48:12 +0100
Subject: [PATCH] Use gsl::narrow for narrowing casts, and document the
 inclusiveness of the scroll rects and the reason for using SHORT_MAX as a
 right boundary.

---
 src/host/getset.cpp                    | 6 ++++--
 src/host/ut_host/ScreenBufferTests.cpp | 8 ++++----
 src/terminal/adapter/adaptDispatch.cpp | 7 +++++--
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/host/getset.cpp b/src/host/getset.cpp
index a95e0c64e53e..d6df0a6424bd 100644
--- a/src/host/getset.cpp
+++ b/src/host/getset.cpp
@@ -1356,7 +1356,8 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool
         if (screenInfo.IsCursorInMargins(oldCursorPosition))
         {
             // Cursor is at the top of the viewport
-            // Rectangle to cut out of the existing buffer
+            // Rectangle to cut out of the existing buffer. This is inclusive.
+            // It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
             SMALL_RECT srScroll;
             srScroll.Left = 0;
             srScroll.Right = SHORT_MAX;
@@ -2030,7 +2031,8 @@ void DoSrvPrivateModifyLinesImpl(const unsigned int count, const bool insert)
     const auto cursorPosition = textBuffer.GetCursor().GetPosition();
     if (screenInfo.IsCursorInMargins(cursorPosition))
     {
-        // Rectangle to cut out of the existing buffer
+        // Rectangle to cut out of the existing buffer. This is inclusive.
+        // It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
         SMALL_RECT srScroll;
         srScroll.Left = 0;
         srScroll.Right = SHORT_MAX;
diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp
index 79b9c7a71160..b0e26b1a68e2 100644
--- a/src/host/ut_host/ScreenBufferTests.cpp
+++ b/src/host/ut_host/ScreenBufferTests.cpp
@@ -3067,7 +3067,7 @@ void _FillLine(COORD position, T fillContent, TextAttribute fillAttr)
 template<class T>
 void _FillLine(int line, T fillContent, TextAttribute fillAttr)
 {
-    _FillLine({ 0, SHORT(line) }, fillContent, fillAttr);
+    _FillLine({ 0, gsl::narrow<SHORT>(line) }, fillContent, fillAttr);
 }
 
 template<class T>
@@ -3101,7 +3101,7 @@ bool _ValidateLineContains(COORD position, T expectedContent, TextAttribute expe
 template<class T>
 bool _ValidateLineContains(int line, T expectedContent, TextAttribute expectedAttr)
 {
-    return _ValidateLineContains({ 0, SHORT(line) }, expectedContent, expectedAttr);
+    return _ValidateLineContains({ 0, gsl::narrow<SHORT>(line) }, expectedContent, expectedAttr);
 }
 
 template<class T>
@@ -3249,8 +3249,8 @@ void ScreenBufferTests::ScrollOperations()
     }
 
     Log::Comment(L"Scrolled area should have moved up/down by given magnitude.");
-    viewportChar += wchar_t(deletedLines); // Characters dropped when deleting
-    viewportLine += SHORT(insertedLines); // Lines skipped when inserting
+    viewportChar += gsl::narrow<wchar_t>(deletedLines); // Characters dropped when deleting
+    viewportLine += gsl::narrow<SHORT>(insertedLines); // Lines skipped when inserting
     while (viewportLine < viewportEnd - deletedLines)
     {
         VERIFY_IS_TRUE(_ValidateLineContains(viewportLine++, viewportChar++, viewportAttr));
diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp
index aed7f4b34ef3..d669533f2bd2 100644
--- a/src/terminal/adapter/adaptDispatch.cpp
+++ b/src/terminal/adapter/adaptDispatch.cpp
@@ -515,7 +515,8 @@ bool AdaptDispatch::_InsertDeleteHelper(_In_ unsigned int const uiCount, const b
     RETURN_IF_FALSE(_conApi->GetConsoleScreenBufferInfoEx(&csbiex));
 
     const auto cursor = csbiex.dwCursorPosition;
-    // Rectangle to cut out of the existing buffer
+    // Rectangle to cut out of the existing buffer. This is inclusive.
+    // It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
     SMALL_RECT srScroll;
     srScroll.Left = cursor.X;
     srScroll.Right = SHORT_MAX;
@@ -955,11 +956,13 @@ bool AdaptDispatch::_ScrollMovement(const ScrollDirection sdDirection, _In_ unsi
 
         if (fSuccess)
         {
+            // Rectangle to cut out of the existing buffer. This is inclusive.
+            // It will be clipped to the buffer boundaries so SHORT_MAX gives us the full buffer width.
             SMALL_RECT srScreen;
             srScreen.Left = 0;
             srScreen.Right = SHORT_MAX;
             srScreen.Top = csbiex.srWindow.Top;
-            srScreen.Bottom = csbiex.srWindow.Bottom - 1;
+            srScreen.Bottom = csbiex.srWindow.Bottom - 1; // srWindow is exclusive, hence the - 1
 
             // Paste coordinate for cut text above
             COORD coordDestination;