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.";