1
\$\begingroup\$

I wrote this function to convert an array of bytes to a C/C++ string representation using hexadecimal escape codes (\xhh). Any suggestions are welcome:

std::string toEscapedHexaString(const std::uint8_t * data, const int dataSizeBytes, 
 const int maxCols = 80, const int padding = 0)
{
 assert(data != nullptr);
 assert((maxCols % 4) == 0);
 assert((padding % 2) == 0);
 int column = 0;
 char hexaStr[64] = {'0円'};
 std::string result = "\"";
 for (int i = 0; i < dataSizeBytes; ++i, ++data)
 {
 std::snprintf(hexaStr, sizeof(hexaStr), "\\x%02X", static_cast<unsigned>(*data));
 result += hexaStr;
 column += 4;
 if (column >= maxCols)
 {
 if (i != (dataSizeBytes - 1)) // If not the last iteration
 {
 result += "\"\n\"";
 }
 column = 0;
 }
 }
 // Add zero padding at the end to ensure the data size
 // is evenly divisible by the given padding value.
 if (padding > 0)
 {
 for (int i = dataSizeBytes; (i % padding) != 0; ++i)
 {
 result += "\\x00";
 column += 4;
 if (column >= maxCols)
 {
 if ((i + 1) % padding) // If not the last iteration
 {
 result += "\"\n\"";
 }
 column = 0;
 }
 }
 }
 result += "\"";
 return result;
}

You can control the number of columns in the output, so for example, converting Hello World!0円 with 20 columns max (not counting the quotes):

const unsigned char str[] = "Hello World!";
auto result = toEscapedHexaString(str, sizeof(str), 20);
/* result = 
 "\x48\x65\x6C\x6C\x6F"
 "\x20\x57\x6F\x72\x6C"
 "\x64\x21\x00"
*/
asked Feb 17, 2016 at 22:22
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Use of assert

Some of the uses of assert here look semi-broken, at least to me. Part of the definition of assert is that when you compile with NDEBUG defined, assert becomes a nop. When it is enabled, a failed assertion immediately aborts the program. For testing inputs to a function, neither of those is generally desirable. For such purposes, I usually define an assure that always does its thing, and throws an exception when the test fails.

snprintf

At the very least, I'd try to wrap the ugliness of using snprintf up into a reasonably neat little function like to_hex that just converts a number into its hex representation:

std::string to_hex(int in) { 
 // snprintf ugliness here
 return std::string(buffer);
}

Alternatively, since you're converting inputs to strings, and accumulating those together into a buffer anyway, consider using an std::ostringstream.

Logic

In a few places, the logic seems...less than pleasing, at least to me. For example:

if (column >= maxCols)

You've previously tested that maxCols % 4 == 0, and (assuming things work correctly) each item you add should be 4 more characters, so you should never see a result that's actually greater than maxCols--only less than maxCols, then equal to maxCols, then back to 0. This is harmless, and if you might want to support a maxCols that was not a multiple of 4, then it might even make good sense--but as things stand right now, it seems like there's a degree of...uncertainty about the real intent.

Reorganization

Right now you have code to ensure the maximum line length sprinkled liberally throughout the rest of the code. I'd be tempted to define a type that takes that as its single responsibility: when you create it, specify a maximum length. When you add characters to it, it tracks the length of the current "line", and when the maximum is reached (or exceeded) it adds a delimiter, and starts over. With this, the rest of the code gets a lot simpler in a hurry, and each piece of code has a much simpler, clearer purpose.

answered Feb 18, 2016 at 2:04
\$\endgroup\$
2
  • \$\begingroup\$ Thanks Jerry, good points! I think you meant to_string instead? stoi is for the other way around, string => integer? Unfortunately to_string doesn't allow specifying a base, so I'm thinking the only other way to avoid snprintf is using a stringstream... \$\endgroup\$ Commented Feb 18, 2016 at 3:11
  • \$\begingroup\$ Oops-yes, stoi was the wrong one. But you're right-I'd forgotten to_string doesn't let you specify the base. \$\endgroup\$ Commented Feb 18, 2016 at 3:44

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.