pw_hex_dump: Fix bug with unaligned trailing bytes
When bytes at the end of a dump were not aligned with the "group_every"
option, half of the last byte would be truncated.
No-Docs-Update-Reason: Bugfix
Change-Id: Ibe349c9f34f41281122583a6e7dff84d4f991333
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/49305
Pigweed-Auto-Submit: Armando Montanez <amontanez@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Armando Montanez <amontanez@google.com>
diff --git a/pw_hex_dump/hex_dump.cc b/pw_hex_dump/hex_dump.cc
index 67f0f3c..77c15cf 100644
--- a/pw_hex_dump/hex_dump.cc
+++ b/pw_hex_dump/hex_dump.cc
@@ -44,6 +44,26 @@
return std::to_integer<char>(b);
}
+void AddGroupingByte(size_t byte_index,
+ FormattedHexDumper::Flags& flags,
+ StringBuilder& builder) {
+ // Never add grouping when it is disabled.
+ if (flags.group_every == 0) {
+ return;
+ }
+ // If this byte isn't at the end of a group, don't add a space.
+ if ((byte_index + 1) % flags.group_every != 0) {
+ return;
+ }
+ // If this byte is the last byte in a line, don't add a grouping byte
+ // (prevents trailing spaces).
+ if (byte_index + 1 == flags.bytes_per_line) {
+ return;
+ }
+
+ builder << ' ';
+}
+
} // namespace
Status DumpAddr(std::span<char> dest, uintptr_t addr) {
@@ -100,17 +120,7 @@
} else {
builder.append(2, ' ');
}
- if (flags.group_every != 0 && (i + 1) % flags.group_every == 0) {
- builder << ' ';
- }
- }
-
- // Removes extraneous space from end when bytes_per_line is divisible by
- // group_every. kSectionSeparator includes a space, so it's unnecessary.
- // Ommitting the space from the section separator actually makes for more
- // workarounds and code duplication, so this is better.
- if (flags.group_every != 0 && flags.bytes_per_line % flags.group_every == 0) {
- builder.pop_back();
+ AddGroupingByte(i, flags, builder);
}
if (flags.show_ascii) {
@@ -173,9 +183,7 @@
// width bytes? (`04` instead of `4`, for example)
builder << std::byte(c >> 4);
builder << std::byte(c & 0xF);
- if (flags.group_every != 0 && (i + 1) % flags.group_every == 0) {
- builder << ' ';
- }
+ AddGroupingByte(i, flags, builder);
}
// Add padding spaces to ensure lines are aligned.
if (flags.show_ascii) {
@@ -183,20 +191,10 @@
i < static_cast<size_t>(flags.bytes_per_line);
++i) {
builder.append(2, ' ');
- if (flags.group_every != 0 && (i + 1) % flags.group_every == 0) {
- builder << ' ';
- }
+ AddGroupingByte(i, flags, builder);
}
}
- // Removes extraneous space from end when bytes_per_line is divisible by
- // group_every. kSectionSeparator includes a space, so it's unnecessary.
- // Ommitting the space from the section separator actually makes for more
- // workarounds and code duplication, so this is better.
- if (flags.group_every != 0 && flags.bytes_per_line % flags.group_every == 0) {
- builder.pop_back();
- }
-
// Interpret bytes as characters.
if (flags.show_ascii) {
builder << kSectionSeparator;
diff --git a/pw_hex_dump/hex_dump_test.cc b/pw_hex_dump/hex_dump_test.cc
index f374431..d283c18 100644
--- a/pw_hex_dump/hex_dump_test.cc
+++ b/pw_hex_dump/hex_dump_test.cc
@@ -223,6 +223,27 @@
EXPECT_STREQ(expected2, dest_.data());
}
+TEST_F(HexDump, FormattedHexDump_LastLineCheck) {
+ constexpr const char* expected1 = "a4cc32629b46381a 231a2a7abce240a0";
+ constexpr const char* expected2 = "ff33e52b9e9f6b3c be9b893c7e4a7a48";
+ constexpr const char* expected3 = "18";
+
+ default_flags_.bytes_per_line = 16;
+ default_flags_.group_every = 8;
+ dumper_ = FormattedHexDumper(dest_, default_flags_);
+
+ EXPECT_TRUE(dumper_.BeginDump(source_data).ok());
+ // Dump first line.
+ EXPECT_TRUE(dumper_.DumpLine().ok());
+ EXPECT_STREQ(expected1, dest_.data());
+ // Dump second line.
+ EXPECT_TRUE(dumper_.DumpLine().ok());
+ EXPECT_STREQ(expected2, dest_.data());
+ // Dump third line.
+ EXPECT_TRUE(dumper_.DumpLine().ok());
+ EXPECT_STREQ(expected3, dest_.data());
+}
+
TEST_F(HexDump, FormattedHexDump_Ascii) {
constexpr const char* expected1 = "6d 79 20 74 65 73 74 20 my test ";
constexpr const char* expected2 = "73 74 72 69 6e 67 0a string.";
