Strip trailing colon from convertMacToString output

Summary:
Fixes #258. MacHelpers.cpp:52
formatted each MAC byte as "{0:02x}:" unconditionally, so
convertMacToString appended a colon after every byte including the
last, producing "aa:bb:cc:dd:ee:ff:" instead of valid MAC notation
"aa:bb:cc:dd:ee:ff". Rewrote the loop to insert the separator only
between bytes (index > 0) instead of building the full string then
truncating, dropping the now-unneeded intermediate locals.

Checked every caller of convertMacToString (KatranGrpcService.cpp,
KatranServiceHandler.cpp Thrift getMac handler) through to their
consumers (katranadm CLI print, go client log line): both are
display-only, nothing parses or string-compares the trailing colon,
so blast radius is limited to the returned string's format. Updated
MacHelpersTest.cpp's 3 hardcoded assertions and the
stringToUintRoundTrip test, which previously codified the trailing
colon (and worked around it with a manual substr) as expected
behavior.

Community PR #259 proposes
the same fix via mac_string.substr(0, mac_string.size() - 1)
(build-full-then-truncate); this diff takes the equivalent
insert-separator-only approach instead to avoid the redundant
allocation and unused intermediate locals, and additionally fixes
the test fixtures the PR's description doesn't mention.

Reviewed By: tagrawal03

Differential Revision: D114403491

fbshipit-source-id: 4d2fd4a5b60121d22644e7498f4e6fc820760434

Author: Ivan Morett

katran/lib/MacHelpers.cpp

@@ -44,13 +44,12 @@ std::string convertMacToString(std::vector<uint8_t> mac) {
   if (mac.size() != 6) {
     return "unknown";
   }
-  uint16_t mac_part;
-  std::string mac_part_string;
   std::string mac_string;
-  for (auto m : mac) {
-    mac_part = m;
-    mac_part_string = fmt::format("{0:02x}:", mac_part);
-    mac_string += mac_part_string;
+  for (size_t i = 0; i < mac.size(); i++) {
+    if (i > 0) {
+      mac_string += ":";
+    }
+    mac_string += fmt::format("{0:02x}", static_cast<uint16_t>(mac[i]));
   }
   return mac_string;
 }

katran/lib/tests/MacHelpersTest.cpp

@@ -50,17 +50,17 @@ TEST(MacHelpersTests, convertMacToUintTooShortReturnsZeroes) {
 
 TEST(MacHelpersTests, convertMacToStringValid) {
   const std::vector<uint8_t> mac{0x00, 0x11, 0x22, 0xaa, 0xbb, 0xcc};
-  EXPECT_EQ(convertMacToString(mac), "00:11:22:aa:bb:cc:");
+  EXPECT_EQ(convertMacToString(mac), "00:11:22:aa:bb:cc");
 }
 
 TEST(MacHelpersTests, convertMacToStringAllZeroes) {
   const std::vector<uint8_t> mac(6, 0);
-  EXPECT_EQ(convertMacToString(mac), "00:00:00:00:00:00:");
+  EXPECT_EQ(convertMacToString(mac), "00:00:00:00:00:00");
 }
 
 TEST(MacHelpersTests, convertMacToStringAllMax) {
   const std::vector<uint8_t> mac(6, 0xff);
-  EXPECT_EQ(convertMacToString(mac), "ff:ff:ff:ff:ff:ff:");
+  EXPECT_EQ(convertMacToString(mac), "ff:ff:ff:ff:ff:ff");
 }
 
 TEST(MacHelpersTests, convertMacToStringWrongSizeReturnsUnknown) {
@@ -75,15 +75,13 @@ TEST(MacHelpersTests, convertMacToStringEmptyReturnsUnknown) {
 }
 
 TEST(MacHelpersTests, stringToUintRoundTrip) {
-  // The string->uint direction is reversible (modulo the trailing colon
-  // that convertMacToString appends). Verifying the byte vector survives
-  // a string->uint->string->uint round-trip catches any silent corruption.
+  // Verifies the byte vector survives a string->uint->string->uint
+  // round-trip, catching any silent corruption.
   const std::string input = "01:23:45:67:89:ab";
   const auto bytes = convertMacToUint(input);
   const auto reformatted = convertMacToString(bytes);
-  EXPECT_EQ(reformatted, input + ":");
-  EXPECT_EQ(
-      convertMacToUint(reformatted.substr(0, reformatted.size() - 1)), bytes);
+  EXPECT_EQ(reformatted, input);
+  EXPECT_EQ(convertMacToUint(reformatted), bytes);
 }
 
 } // namespace katran