Printing hex dumps for diagnostics I have developed some free standing functions to provide hex dumps for buffer (std::string) contents.

You can see it working in action here.

The purpose is for debugging diagnostics to print buffer contents to a (logging) stream, in a format similar as wireshark does.

Any proposals for improvements are welcome.

Here's the code:

#include &lt;iostream&gt;
#include &lt;iomanip&gt;
#include &lt;cctype&gt;

std::ostream&amp; render_printable_chars(std::ostream&amp; os, const char* buffer, size_t bufsize) {
    os &lt;&lt; " | ";
    for (size_t i = 0; i &lt; bufsize; ++i)
    {
        if (std::isprint(buffer[i]))
        {
            os &lt;&lt; buffer[i];
        }
        else
        {
            os &lt;&lt; ".";
        }
    }
    return os;
}

std::ostream&amp; hex_dump(std::ostream&amp; os, const uint8_t* buffer, size_t bufsize, bool showPrintableChars = true)
{
    auto oldFormat = os.flags();
    auto oldFillChar = os.fill();

    os &lt;&lt; std::hex;
    os.fill('0');
    bool printBlank = false;
    size_t i = 0;
    for (; i &lt; bufsize; ++i)
    {
        if (i % 8 == 0)
        {
            if (i != 0 &amp;&amp; showPrintableChars)
            {
                render_printable_chars(os, reinterpret_cast&lt;const char*&gt;(&amp;buffer[i] - 8), 8);
            }
            os &lt;&lt; std::endl;
            printBlank = false;
        }
        if (printBlank)
        {
            os &lt;&lt; ' ';
        }
        os &lt;&lt; std::setw(2) &lt;&lt; std::right &lt;&lt; unsigned(buffer[i]);
        if (!printBlank)
        {
            printBlank = true;
        }
    }
    if (i % 8 != 0 &amp;&amp; showPrintableChars)
    {
        for (size_t j = 0; j &lt; 8 - (i % 8); ++j)
        {
            os &lt;&lt; "   ";
        }
        render_printable_chars(os, reinterpret_cast&lt;const char*&gt;(&amp;buffer[i] - (i % 8)), (i % 8));
    }

    os &lt;&lt; std::endl;

    os.fill(oldFillChar);
    os.flags(oldFormat);

    return os;
}

std::ostream&amp; hex_dump(std::ostream&amp; os, const std::string&amp; buffer, bool showPrintableChars = true)
{
    return hex_dump(os,reinterpret_cast&lt;const uint8_t*&gt;(buffer.data()), buffer.length(),showPrintableChars);
}




Here's the test case:

int main()
{
    const char test[] = "abcdef123456\0zyxwvu987654";
    std::string s(test,sizeof(test));

    hex_dump(std::cout, s);

    return 0;
}


The output looks like expected:

61 62 63 64 65 66 31 32 | abcdef12
33 34 35 36 00 7a 79 78 | 3456.zyx
77 76 75 39 38 37 36 35 | wvu98765
34 00                   | 4.
Did you ever test it with buffers which are multiples of 8 long?
Because the last block won't get the nice printable display.
Are you sure the newline at the beginning of the output should be there?
Should the test-output really contain the implicit 0-terminator?
return 0; is implicit for main().


Modified test-function:

int main()
{
    const char test[] = "abcdef123456\0zyxwvu987654";
    std::string s(test, sizeof(test) - 1);

    std::cout &lt;&lt; "---\n";
    hex_dump(std::cout, s);
    std::cout &lt;&lt; "---\n";
    hex_dump(std::cout, "");
    std::cout &lt;&lt; "---\n";
    hex_dump(std::cout, "12345678");
    std::cout &lt;&lt; "---\n";
    hex_dump(std::cout, "1234567812345678");
    std::cout &lt;&lt; "---\n";
}



Do you have the opening brace for functions on its own line or not? Seems 2+1 for, 1 against...
If you only want to output a single character, using a character is potentially more efficient than a string.
The conditional operator is excellent for choosing between two values.
Beware of the implementation-defined signedness of plain char. The character-classification-functions inherited from C expect the value of an unsigned char or EOF.
See std::isprint.
Consider marking internal functions static to avoid externally visible symbols and promote inlining.
If you don't actually ever use a return-value, why provide one?


static void render_printable_chars(std::ostream&amp; os, const char* buffer, size_t bufsize)
{
    os &lt;&lt; " | ";
    for (size_t i = 0; i &lt; bufsize; ++i)
        os &lt;&lt; (std::isprint((unsigned char)buffer[i]) ? buffer[i] : '.');
}



Don't play around with a useless flag just because. Tracing data-flow is quite a lot more involved than following the program-flow.
Testing whether a variable already has a value before setting it to the same is rarely anything but obfuscation and pessimisation.
Don't use std::endl unless you really want to flush. And if that's your intention, consider being more explicit by using std::flush.
If you use C++17, you might want to accept a std::string_view by value instead of a std::string by constant reference. It's generally more efficient.