# Format raw bytes in hex and ascii in manner like Wireshark does

This function formats raw buffer output in manner as Wireshark and many others does:

0x00: 68 65 6C 6C 6F 20 77 6F 72 6C 64 02 6B 68 67 61   |hello world.khga|
0x10: 76 73 64 20 0B 20 0A 05 58 61 73 6A 68 6C 61 73   |vsd . ..Xasjhlas|
0x20: 62 64 61 73 20 6A 61 6C 73 6A 64 6E 13 20 20 30   |bdas jalsjdn.  0|
0x30: 31 32 33 34 35 36 37 38 39                        |123456789       |


Please review and assess code quality.

std::string format_hex_payload(const char* payload, size_t payload_len)
{
const char line_placeholder[] = "0x00:                                                   |                |";
size_t sizeof_output = sizeof line_placeholder * number_of_lines;
char *output = (char*)alloca(sizeof_output);  //POSSIBLE REIMPL USE C++RVO std::string out; out.reserve(sizeof_output);

char *pout = output;  //POSSIBLE REIMPL char *pout = out.data();
size_t ascii_offset = strchr(line_placeholder,'|') - line_placeholder + 1;  //could be calculated at compile time

unsigned short offset = 0;
for(unsigned l=0; l < number_of_lines; l++, offset+=16)
{
char* pline_begin = pout;
char* pline = pout;
strcpy(pline,line_placeholder);
pline += sprintf(pline, "0x%02X: ", offset);
for(unsigned i=0; i<16 && p < end; ++i, ++p){
pline += sprintf(pline, "%02X ", *p);
*(pline_begin+ascii_offset+i) = isprint(*p) ? *p : '.';
}
*pline=' ';
pout += sizeof line_placeholder;  // move pointer to next line
pout[-1] = '\n';
}
pout[-1] = '\0';
assert(pout == output + sizeof_output);  // sanity check

return output;  //POSSIBLE REIMPL return out;
}


• What happens when offset is >= 256 (more than 16 lines)? – 1201ProgramAlarm Feb 7 at 19:11
• @1201ProgramAlarm, nothing criminal, but format will be shifted i.imgur.com/rlMNjc0.png – kyb Feb 11 at 11:23
• @kyb: The policy on this site is not to add, remove, or edit code in a question after you've received an answer, because it invalidates existing answers. This is explained in What to do when someone answers. On the other hand, your original question was unclear/off-topic (tagged [C] but invalid C code), which is probably why you code 3 close votes up to now. – Martin R Feb 11 at 12:33
• @kyb: (Cont.) Following feedback in chat, I have done a rollback the question to revision 3, i.e. the original code but with the [C++] tag, so that both Q and A make sense. I suggest to ask a new question about the C implementation. – Martin R Feb 11 at 12:33
• Follow up question (Compiles to C rather than C++ now) – Peilonrayz Feb 11 at 15:39

1. I think probably the first thing to do is decide whether you're really using C, or really using C++.

• If you're really using C, then you need to get rid of the std::string, and allocate space for your return differently (e.g., using malloc).

• If you're really using C++, then I'd at least consider using iostreams and manipulators to do most of the work. They are fairly verbose, but most people using C++ expect to deal with iostreams rather than raw buffers and C-style string manipulation.

2. I think I'd break the code up into a few more functions. For example, I'd probably have format_hex, which would repeatedly call format_line, which might in turn call some format_hex_value (or something on that order) to write out each individual value.

3. I'd probably avoid using the line placeholder, for a couple of reasons. The first (and probably most important) is that as you've done things right now, the last line is (as I see things) somewhat malformed. You've surrounded the ASCII part of the display with | as delimiters, but for that to make sense, you want the delimiter right next to the data, so that part of the last line should look like: |123456789|, so you immediately know there's no more data after the 9 (where your current display looks like there are space characters after the 9).

4. I'd definitely expand the addresses on the left out to at least 4 characters, and maybe more. In theory, you should probably be thinking in terms of 16 characters (allowing 64-bit addresses), but in all honesty I've rarely seen much point in hex dump of multiple gigabytes of data (but I certainly have seen good uses for more than 256 bytes).

Putting those together, one possibility (going the C++ route) might come out something on this order:

#include <cctype>
#include <iostream>
#include <iomanip>
#include <sstream>

struct hex {
unsigned value;
int digits;
public:
hex(unsigned value, int digits) : value(value), digits(digits) { }

friend std::ostream &operator<<(std::ostream &os, hex const &h) {
auto oldfill = os.fill('0');

// Yeah, iostreams get really verbose. Sorry.
os << std::hex
<< std::setw(h.digits)
<< std::uppercase
<< std::setprecision(h.digits)
<< h.value;
os.fill(oldfill);
return os;
}
};

void fmt_line(std::ostream &os, char const *data, size_t offset, size_t len) {
os << hex(offset, 8) << ":";

for (size_t i=0; i<len; i++) {
os << " " << hex(data[offset+i], 2);
}

os << std::setw((16-len)*3 +2) << "|";
for (size_t i=0; i<len; i++) {
char ch = data[offset+i];
os << (std::isprint((unsigned char)ch) ? ch : '.');
}
os << "|\n";
}

std::string format_hex(char const *data, size_t len) {
unsigned lines = len/16;
std::stringstream out;

for (size_t line=0; line<lines; line++)
fmt_line(out, data, line*16, 16);
fmt_line(out, data, lines*16, len%16);
return out.str();
}

int main() {
char input[] = "hello world\02khgavsd \xb \xa\x5Xasjhlasbdas jalsjdn\xd  0123456789";
std::cout << format_hex(input, sizeof(input));
}


This could use some further improvement. For one thing, it's probably not immediately obvious what some things like: os << std::setw((16-len)*3 +2) << "|"; are really doing, so they could undoubtedly use at least a little more work.