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"
*/
\$\endgroup\$
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.

\$\endgroup\$
  • \$\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\$ – glampert Feb 18 '16 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\$ – Jerry Coffin Feb 18 '16 at 3:44

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.