octoclock: Avoid usage of uninitialized memory

The Octoclock host code would send uninitialized memory over the
network, which would be flagged by tools such as Valgrind. This patch
creates a factory function for OctoClock packets that initializes the
memory to zero, defaults the proto version to the OctoClock default, and
can provide a random sequence number if none is given.

5 files changed, 48 insertions, 41 deletions

diff --git a/host/lib/usrp_clock/octoclock/octoclock_eeprom.cpp b/host/lib/usrp_clock/octoclock/octoclock_eeprom.cpp
index 349d704f3..ad6d9c7db 100644
--- a/host/lib/usrp_clock/octoclock/octoclock_eeprom.cpp
+++ b/host/lib/usrp_clock/octoclock/octoclock_eeprom.cpp
@@ -6,6 +6,7 @@
 //
 
 #include "common.h"
+#include "octoclock_impl.hpp"
 #include <uhd/exception.hpp>
 #include <uhd/transport/udp_simple.hpp>
 #include <uhd/types/byte_vector.hpp>
@@ -15,7 +16,6 @@
 #include <uhd/utils/byteswap.hpp>
 #include <boost/asio.hpp>
 #include <boost/assign/list_of.hpp>
-#include <iostream>
 
 typedef boost::asio::ip::address_v4 ip_v4;
 
@@ -34,11 +34,8 @@ void octoclock_eeprom_t::_load()
     const octoclock_fw_eeprom_t* eeprom_in =
         reinterpret_cast<const octoclock_fw_eeprom_t*>(pkt_in->data);
 
-    octoclock_packet_t pkt_out;
-    // To avoid replicating sequence numbers between sessions
-    pkt_out.sequence = uint32_t(std::rand());
-    size_t len       = 0;
-
+    auto pkt_out = make_octoclock_packet();
+    size_t len   = 0;
     UHD_OCTOCLOCK_SEND_AND_RECV(
         xport, _proto_ver, SEND_EEPROM_CMD, pkt_out, len, octoclock_data);
     if (UHD_OCTOCLOCK_PACKET_MATCHES(SEND_EEPROM_ACK, pkt_out, pkt_in, len)) {
@@ -86,11 +83,9 @@ void octoclock_eeprom_t::_store() const
     const octoclock_packet_t* pkt_in =
         reinterpret_cast<const octoclock_packet_t*>(octoclock_data);
 
-    octoclock_packet_t pkt_out;
-    // To avoid replicating sequence numbers between sessions
-    pkt_out.sequence = uint32_t(std::rand());
-    pkt_out.len      = sizeof(octoclock_fw_eeprom_t);
-    size_t len       = 0;
+    auto pkt_out = make_octoclock_packet();
+    pkt_out.len  = sizeof(octoclock_fw_eeprom_t);
+    size_t len   = 0;
 
     octoclock_fw_eeprom_t* eeprom_out =
         reinterpret_cast<octoclock_fw_eeprom_t*>(&pkt_out.data);
diff --git a/host/lib/usrp_clock/octoclock/octoclock_image_loader.cpp b/host/lib/usrp_clock/octoclock/octoclock_image_loader.cpp
index 02a243b70..c588404aa 100644
--- a/host/lib/usrp_clock/octoclock/octoclock_image_loader.cpp
+++ b/host/lib/usrp_clock/octoclock/octoclock_image_loader.cpp
@@ -168,7 +168,7 @@ static void octoclock_setup_session(octoclock_session_t& session,
     session.sequence = uint32_t(std::rand());
 
     // Query OctoClock again to get compat number
-    octoclock_packet_t pkt_out;
+    auto pkt_out = make_octoclock_packet();
     const octoclock_packet_t* pkt_in =
         reinterpret_cast<const octoclock_packet_t*>(session.data_in);
     size_t len = 0;
@@ -192,8 +192,7 @@ static void octoclock_reset_into_bootloader(octoclock_session_t& session)
         return;
 
     // Force compat num to device's current, works around old firmware bug
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence  = uhd::htonx<uint32_t>(++session.sequence);
+    auto pkt_out      = make_octoclock_packet(uhd::htonx<uint32_t>(++session.sequence));
     pkt_out.proto_ver = uhd::htonx<uint32_t>(session.starting_firmware_version);
     pkt_out.code      = RESET_CMD;
 
@@ -229,8 +228,7 @@ static void octoclock_burn(octoclock_session_t& session)
     // Make sure we're in the bootloader for this
     octoclock_reset_into_bootloader(session);
 
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence = uhd::htonx<uint32_t>(++session.sequence);
+    auto pkt_out = make_octoclock_packet(uhd::htonx<uint32_t>(++session.sequence));
     const octoclock_packet_t* pkt_in =
         reinterpret_cast<const octoclock_packet_t*>(session.data_in);
 
@@ -284,8 +282,7 @@ static void octoclock_burn(octoclock_session_t& session)
 
 static void octoclock_verify(octoclock_session_t& session)
 {
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence = uhd::htonx<uint32_t>(++session.sequence);
+    auto pkt_out = make_octoclock_packet(uhd::htonx<uint32_t>(++session.sequence));
     const octoclock_packet_t* pkt_in =
         reinterpret_cast<const octoclock_packet_t*>(session.data_in);
     size_t len = 0;
@@ -331,8 +328,7 @@ static void octoclock_verify(octoclock_session_t& session)
 
 static void octoclock_finalize(octoclock_session_t& session)
 {
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence = uhd::htonx<uint32_t>(++session.sequence);
+    auto pkt_out = make_octoclock_packet(uhd::htonx<uint32_t>(++session.sequence));
     const octoclock_packet_t* pkt_in =
         reinterpret_cast<const octoclock_packet_t*>(session.data_in);
     size_t len = 0;
diff --git a/host/lib/usrp_clock/octoclock/octoclock_impl.cpp b/host/lib/usrp_clock/octoclock/octoclock_impl.cpp
index b7ebc3473..146470a7b 100644
--- a/host/lib/usrp_clock/octoclock/octoclock_impl.cpp
+++ b/host/lib/usrp_clock/octoclock/octoclock_impl.cpp
@@ -99,12 +99,9 @@ device_addrs_t octoclock_find(const device_addr_t& hint)
         _hint["addr"], BOOST_STRINGIZE(OCTOCLOCK_UDP_CTRL_PORT));
 
     // Send a query packet
-    octoclock_packet_t pkt_out;
-    pkt_out.proto_ver = OCTOCLOCK_FW_COMPAT_NUM;
-    // To avoid replicating sequence numbers between sessions
-    pkt_out.sequence = uint32_t(std::rand());
-    pkt_out.len      = 0;
-    pkt_out.code     = OCTOCLOCK_QUERY_CMD;
+    auto pkt_out = make_octoclock_packet();
+    pkt_out.len  = 0;
+    pkt_out.code = OCTOCLOCK_QUERY_CMD;
     try {
         udp_transport->send(boost::asio::buffer(&pkt_out, sizeof(pkt_out)));
     } catch (const std::exception& ex) {
@@ -180,6 +177,27 @@ UHD_STATIC_BLOCK(register_octoclock_device)
 }
 
 /***********************************************************************
+ * Helpers
+ **********************************************************************/
+octoclock_packet_t make_octoclock_packet()
+{
+    octoclock_packet_t new_pkt;
+    memset(&new_pkt, 0, sizeof(octoclock_packet_t));
+    new_pkt.sequence  = uint32_t(std::rand());
+    new_pkt.proto_ver = OCTOCLOCK_FW_COMPAT_NUM;
+    return new_pkt;
+}
+
+octoclock_packet_t make_octoclock_packet(const uint32_t sequence)
+{
+    octoclock_packet_t new_pkt;
+    memset(&new_pkt, 0, sizeof(octoclock_packet_t));
+    new_pkt.sequence  = sequence;
+    new_pkt.proto_ver = OCTOCLOCK_FW_COMPAT_NUM;
+    return new_pkt;
+}
+
+/***********************************************************************
  * Structors
  **********************************************************************/
 octoclock_impl::octoclock_impl(const device_addr_t& _device_addr)
@@ -350,9 +368,7 @@ void octoclock_impl::_set_eeprom(
 
 uint32_t octoclock_impl::_get_fw_version(const std::string& oc)
 {
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence = uhd::htonx<uint32_t>(++_sequence);
-    pkt_out.len      = 0;
+    auto pkt_out = make_octoclock_packet(uhd::htonx<uint32_t>(++_sequence));
     size_t len;
 
     uint8_t octoclock_data[udp_simple::mtu];
@@ -373,10 +389,8 @@ uint32_t octoclock_impl::_get_fw_version(const std::string& oc)
 
 void octoclock_impl::_get_state(const std::string& oc)
 {
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence = uhd::htonx<uint32_t>(++_sequence);
-    pkt_out.len      = 0;
-    size_t len       = 0;
+    auto pkt_out = make_octoclock_packet(uhd::htonx<uint32_t>(++_sequence));
+    size_t len   = 0;
 
     uint8_t octoclock_data[udp_simple::mtu];
     const octoclock_packet_t* pkt_in =
diff --git a/host/lib/usrp_clock/octoclock/octoclock_impl.hpp b/host/lib/usrp_clock/octoclock/octoclock_impl.hpp
index d293ce3e5..01c1f91eb 100644
--- a/host/lib/usrp_clock/octoclock/octoclock_impl.hpp
+++ b/host/lib/usrp_clock/octoclock/octoclock_impl.hpp
@@ -21,6 +21,11 @@
 
 uhd::device_addrs_t octoclock_find(const uhd::device_addr_t& hint);
 
+//! Create an empty octoclock packet with a random sequence number
+octoclock_packet_t make_octoclock_packet();
+//! Create an empty octoclock packet with a given sequence number
+octoclock_packet_t make_octoclock_packet(const uint32_t sequence);
+
 /*!
  * OctoClock implementation guts
  */
diff --git a/host/lib/usrp_clock/octoclock/octoclock_uart.cpp b/host/lib/usrp_clock/octoclock/octoclock_uart.cpp
index 15c503888..4f4ad1e55 100644
--- a/host/lib/usrp_clock/octoclock/octoclock_uart.cpp
+++ b/host/lib/usrp_clock/octoclock/octoclock_uart.cpp
@@ -7,6 +7,7 @@
 
 #include "octoclock_uart.hpp"
 #include "common.h"
+#include "octoclock_impl.hpp"
 #include <uhd/exception.hpp>
 #include <uhd/utils/byteswap.hpp>
 #include <stdint.h>
@@ -42,9 +43,7 @@ octoclock_uart_iface::octoclock_uart_iface(udp_simple::sptr udp, uint32_t proto_
     size_t len = 0;
 
     // Get pool size from device
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence = uhd::htonx<uint32_t>(_sequence);
-    pkt_out.len      = 0;
+    auto pkt_out = make_octoclock_packet(uhd::htonx<uint32_t>(_sequence));
 
     uint8_t octoclock_data[udp_simple::mtu];
     const octoclock_packet_t* pkt_in =
@@ -63,9 +62,8 @@ void octoclock_uart_iface::write_uart(const std::string& buf)
 {
     size_t len = 0;
 
-    octoclock_packet_t pkt_out;
-    pkt_out.sequence = uhd::htonx<uint32_t>(++_sequence);
-    pkt_out.len      = buf.size();
+    auto pkt_out = make_octoclock_packet(uhd::htonx<uint32_t>(++_sequence));
+    pkt_out.len  = buf.size();
     memcpy(pkt_out.data, buf.c_str(), buf.size());
 
     uint8_t octoclock_data[udp_simple::mtu];
@@ -108,8 +106,7 @@ std::string octoclock_uart_iface::read_uart(double timeout)
 
 void octoclock_uart_iface::_update_cache()
 {
-    octoclock_packet_t pkt_out;
-    pkt_out.len = 0;
+    auto pkt_out = make_octoclock_packet();
     size_t len  = 0;
 
     uint8_t octoclock_data[udp_simple::mtu];