Fix some nits in FPDFText_GetText().

Use more variables to avoid redundant calculations. Add one more edge
test case.

Change-Id: I6c8a0aca9de3bdd1a394c39304fd9a75009f9489
Reviewed-on: https://pdfium-review.googlesource.com/19690
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

2 files changed, 21 insertions, 15 deletions

diff --git a/fpdfsdk/fpdftext.cpp b/fpdfsdk/fpdftext.cpp
index 022b7caa8d..5a2deb9a81 100644
--- a/fpdfsdk/fpdftext.cpp
+++ b/fpdfsdk/fpdftext.cpp
@@ -164,22 +164,21 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFText_GetText(FPDF_TEXTPAGE page,
                                                int char_start,
                                                int char_count,
                                                unsigned short* result) {
-  if (char_start < 0 || char_count < 0 || !result || !page)
+  if (!page || char_start < 0 || char_count < 0 || !result)
     return 0;
 
   CPDF_TextPage* textpage = CPDFTextPageFromFPDFTextPage(page);
-  if (char_start >= textpage->CountChars())
+  int char_available = textpage->CountChars() - char_start;
+  if (char_available <= 0)
     return 0;
 
-  char_count = std::min(char_count, textpage->CountChars() - char_start);
+  char_count = std::min(char_count, char_available);
   if (char_count == 0) {
-    // Writting out ""
-    *result = 0;
+    // Writing out "", which has a character count of 1 due to the NUL.
+    *result = '\0';
     return 1;
   }
 
-  int char_last = char_start + char_count - 1;
-
   // char_* values are for a data structure that includes non-printing unicode
   // characters, where the text_* values are from a data structure that doesn't
   // include these characters, so translation is needed.
@@ -187,13 +186,14 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFText_GetText(FPDF_TEXTPAGE page,
   if (text_start == -1)
     return 0;
 
+  int char_last = char_start + char_count - 1;
   int text_last = textpage->TextIndexFromCharIndex(char_last);
   if (text_last == -1)
     return 0;
 
-  if (text_start > text_last)
-    return 0;
   int text_count = text_last - text_start + 1;
+  if (text_count < 1)
+    return 0;
 
   WideString str = textpage->GetPageText(text_start, text_count);
   if (str.GetLength() > static_cast<size_t>(text_count))
@@ -202,12 +202,12 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFText_GetText(FPDF_TEXTPAGE page,
   // UFT16LE_Encode doesn't handle surrogate pairs properly, so it is expected
   // the number of items to stay the same.
   ByteString byte_str = str.UTF16LE_Encode();
-  ASSERT((byte_str.GetLength()) / kBytesPerCharacter <=
-         static_cast<size_t>(char_count + 1));  // +1 to account for the string
-                                                // terminator
-  memcpy(result, byte_str.GetBuffer(byte_str.GetLength()),
-         byte_str.GetLength());
-  return (byte_str.GetLength() / kBytesPerCharacter);
+  size_t byte_str_len = byte_str.GetLength();
+  int ret_count = byte_str_len / kBytesPerCharacter;
+
+  ASSERT(ret_count <= char_count + 1);  // +1 to account for the NUL terminator.
+  memcpy(result, byte_str.GetBuffer(byte_str_len), byte_str_len);
+  return ret_count;
 }
 
 FPDF_EXPORT int FPDF_CALLCONV FPDFText_CountRects(FPDF_TEXTPAGE text_page,
diff --git a/fpdfsdk/fpdftext_embeddertest.cpp b/fpdfsdk/fpdftext_embeddertest.cpp
index 4b82dc081e..51216b9818 100644
--- a/fpdfsdk/fpdftext_embeddertest.cpp
+++ b/fpdfsdk/fpdftext_embeddertest.cpp
@@ -49,6 +49,12 @@ TEST_F(FPDFTextEmbeddertest, Text) {
   EXPECT_EQ(1, FPDFText_GetText(textpage, 0, 0, fixed_buffer));
   EXPECT_EQ(0, fixed_buffer[0]);
 
+  // Keep going and check the next case.
+  memset(fixed_buffer, 0xbd, sizeof(fixed_buffer));
+  EXPECT_EQ(2, FPDFText_GetText(textpage, 0, 1, fixed_buffer));
+  EXPECT_EQ(expected[0], fixed_buffer[0]);
+  EXPECT_EQ(0, fixed_buffer[1]);
+
   // Check includes the terminating NUL that is provided.
   int num_chars = FPDFText_GetText(textpage, 0, 128, fixed_buffer);
   ASSERT_GE(num_chars, 0);