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>
diff --git a/fpdfsdk/fpdftext.cpp b/fpdfsdk/fpdftext.cpp
index 022b7ca..5a2deb9 100644
--- a/fpdfsdk/fpdftext.cpp
+++ b/fpdfsdk/fpdftext.cpp
@@ -164,22 +164,21 @@
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 @@
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 @@
// 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 4b82dc0..51216b9 100644
--- a/fpdfsdk/fpdftext_embeddertest.cpp
+++ b/fpdfsdk/fpdftext_embeddertest.cpp
@@ -49,6 +49,12 @@
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);
