Skip to content

Conversation

@abhina-sree
Copy link
Contributor

This patch addresses the follow-up comments on PR #138893

@abhina-sree abhina-sree changed the title Improvments to TextEncodingConverter Improvements to TextEncodingConverter Jun 2, 2025
@abhina-sree abhina-sree self-assigned this Jun 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 2, 2025

@llvm/pr-subscribers-llvm-support

Author: Abhina Sree (abhina-sree)

Changes

This patch addresses the follow-up comments on PR #138893


Full diff: https://github.com/llvm/llvm-project/pull/142476.diff

1 Files Affected:

  • (modified) llvm/lib/Support/TextEncoding.cpp (+5-5)
diff --git a/llvm/lib/Support/TextEncoding.cpp b/llvm/lib/Support/TextEncoding.cpp
index 969dd419ede72..72a011c85e0eb 100644
--- a/llvm/lib/Support/TextEncoding.cpp
+++ b/llvm/lib/Support/TextEncoding.cpp
@@ -164,7 +164,7 @@ TextEncodingConverterICU::convertString(StringRef Source,
     EC = U_ZERO_ERROR;
     const char *Input = In;
 
-    Output = InputLength ? static_cast<char *>(Result.data()) : nullptr;
+    Output = static_cast<char *>(Result.data());
     ucnv_convertEx(&*ToConvDesc, &*FromConvDesc, &Output, Result.end(), &Input,
                    In + InputLength, /*pivotStart=*/NULL,
                    /*pivotSource=*/NULL, /*pivotTarget=*/NULL,
@@ -175,8 +175,10 @@ TextEncodingConverterICU::convertString(StringRef Source,
         if (Capacity < Result.max_size()) {
           HandleOverflow(Capacity, Output, OutputLength, Result);
           continue;
-        } else
+        } else {
+          Result.resize(Output - Result.data());
           return std::error_code(E2BIG, std::generic_category());
+        }
       }
       // Some other error occured.
       Result.resize(Output - Result.data());
@@ -271,10 +273,8 @@ TextEncodingConverterIconv::convertString(StringRef Source,
   };
 
   do {
-    // Setup the input. Use nullptr to reset iconv state if input length is
-    // zero.
     size_t InputLength = Source.size();
-    char *Input = InputLength ? const_cast<char *>(Source.data()) : "";
+    char *Input = const_cast<char *>(Source.data());
     Ret = iconv(ConvDesc, &Input, &InputLength, &Output, &OutputLength);
     if (Ret != 0) {
       if (auto EC = HandleError(Ret))

Copy link
Collaborator

@hubert-reinterpretcast hubert-reinterpretcast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@abhina-sree abhina-sree force-pushed the abhina/improve_textencodingconverter branch from 057be53 to 92f64ff Compare September 25, 2025 12:32
@abhina-sree abhina-sree merged commit 17c93d6 into llvm:main Sep 25, 2025
9 checks passed
@abhina-sree abhina-sree deleted the abhina/improve_textencodingconverter branch September 25, 2025 13:35
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
This patch addresses the follow-up comments on PR
llvm#138893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants