4
\$\begingroup\$

I created a function that truncates an incomplete UTF-8 character at the end of std::string in C++.

C++'s Standard Library does not yet support character based substr on UTF-8 characters and does substr by number of bytes only.

Because of that, in the example below, substr causes a weird broken character to appear in the end.

std::string utfstr = "옷三옷白옷옷-어<어<어<어<-";
std::cout << utfstr.substr(0, 5) << std::endl;

It seems like my function is working, but, I would like to get some feedback on possible problems and improvements.

#include <string>
#include <iostream>
using namespace std;

ssize_t TrimEndUTF8(std::string& str) {
  // Scans backward from the end of string.
  const char* cptr = &str.back();
  int num = 1;
  int numBytesToTruncate = 0;

  for (int i = 0; 6 > i; ++i) {
    numBytesToTruncate += 1;
    if ((*cptr & 0x80) == 0x80) { // If char bit starts with 1xxxxxxx
      // It's a part of unicode character!
      // Find the first byte in the unicode character!

      //if ((*cptr & 0xFC) == 0xFC) { if (num == 6) { return 0; } break; }
      //if ((*cptr & 0xF8) == 0xF8) { if (num == 5) { return 0; } break; }

      // If char binary is 11110000, it means it's a 4 bytes long unicode.
      if ((*cptr & 0xF0) == 0xF0) { if (num == 4) { return 0; } break; }
      // If char binary is 11100000, it means it's a 3 bytes long unicode.
      if ((*cptr & 0xE0) == 0xE0) { if (num == 3) { return 0; } break; }
      if ((*cptr & 0xC0) == 0xC0) { if (num == 2) { return 0; } break; }  

      num += 1;
    } else {
      // If char bit does not start with 1, nothing to truncate!
      return 0;
    }

    cptr -= 1;
  }
  str.resize(str.length() - numBytesToTruncate);
  return numBytesToTruncate;
}


int main() {
  for (int i = 1; 30 > i; ++i) {
    std::string utfStr = "안-녕<하>세d요e만f나g서반갑습니다";
    std::string substred = utfStr.substr(0, i);
    size_t trimmed = TrimEndUTF8(substred);
    cout << "Trimmed " << trimmed << " bytes" << endl;
    cout << substred << endl;
  }

  for (int i = 1; 30 > i; ++i) {
    std::string utfStr = "𠜎_𠜱_𠝹_𠱓_𠱸_𠲖𠳏𠳕𠴕𠵼-𠵿-𠸎-𠸏-𠹷-𠺝-𠺢𠻗";
    std::string substred = utfStr.substr(0, i);
    size_t trimmed = TrimEndUTF8(substred);
    cout << "Trimmed " << trimmed << " bytes" << endl;
    cout << substred << endl;
  }

  return 0;
}
\$\endgroup\$
3
\$\begingroup\$

There is currently no error checking to validate that this is valid UTF-8. If this is deliberate it should be documented.

You are testing the 1's at the top of a UTF-8 encoded string. But not correctly validating the trailing 0.

  if ((*cptr & 0xC0) == 0x80) { ...
  if ((*cptr & 0xF8) == 0xF0) { ...
  if ((*cptr & 0xF0) == 0xE0) { ...
  if ((*cptr & 0xE0) == 0xC0) { ...
         ///   ^^^^  You should extend the mask one more bit
         ///         So that your test validates that the next
         ///         bit is zero. 

The way your function is written it will mutate the string if it finds a partial. This may not always be necessary I would break this into two functions. One that finds the amount of mutation needed and another that does the mutation. That way another user can potentially use only the non mutating part and get better performance (say if they were passing it to a function using iterators).

 std::size_t trimEndUTF8(std::string& str) {
     std::size_t trim = findEndUTF8Trim(str);
     str.resize(str.size() - trim);
     return trim;
 }

If you implement the above function split. You now make it easy for your self to implement a slight return optimization.

 if ((*cptr & 0xF8) == 0xF0) {
     if (num == 4) {
         return 0;
     }

     // break;
     // Change your break into a return.
     //
     // Because we are not mutating we don't need to worry about
     // changing the input in thus function and can just return the
     // size of the trim. Since this is the head character of a UTF-8
     // sequence the next character must be end of a UTF-8 character.
     //
     return num; // or is it num + 1 you should check.
 }

Prefer not to use yoda conditionals.

 for (int i = 0; 6 > i; ++i)
                 ^^^^^  Yoda conditional

They are a cognitive drain and less intuitive to read (unless you are yoda). Prefer the more traditional style as it makes it easy to read fof developers.

 for (int i = 0; i < 6; ++i)

Note you are assuming there are at least 6 characters in the string. I would not do that (especially since people are likely to throw all kinds of crap at it). A badly formed UTF-8 string is going to cause undefined behavior.

Standard warning about:

using namespace std;

Read any other C++ review on this site. We say don't use this. Read another review and you will find why.

Naming conventions. It is more traditional to use an initial lowercase letter for objects(this includes functions/methods).

ssize_t findEndUTF8Trim(std::string const& str) {

    // If the last character is a single character return
    // Don't need to do any further checking or trimming.
    if ((str.back() & 0x80) == 0x00)
        return 0;
    }

    // Check that we have a valid UTF-8 sequence.
    char const* cptr = &str.back();
    int const   end  = std::min(6, str.size()); // Max theoritical len == 6
    for (int num = 1; num <= end; ++num, --cptr) {
        if ((*cptr & 0xF8) == 0xF0) { return (num == 4) ? 0 : num; } 
        if ((*cptr & 0xF0) == 0xE0) { return (num == 3) ? 0 : num; }
        if ((*cptr & 0xE0) == 0xC0) { return (num == 2) ? 0 : num; }  
    }
    // If reach here then there is a malformed UTF-8 character sequence.
    throw std::runtime_error("Unexpected UTF-8 string");
}
\$\endgroup\$
  • \$\begingroup\$ Great! Extremely helpful. Thank you very much for your effort and time to post the answer. :) \$\endgroup\$ – LETs Jul 23 '15 at 16:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.