diff --git a/src/daqd/CMakeLists.txt b/src/daqd/CMakeLists.txt
index e56dc1f51cf3e14939dfe38db9af777246c0bc67..3cc06ce4bc4e87b242aab543735b9d24a7fe3515 100644
--- a/src/daqd/CMakeLists.txt
+++ b/src/daqd/CMakeLists.txt
@@ -157,10 +157,13 @@ add_executable(test_daqd_unit_tests tests/test_main.cc
tests/test_daqd_cmask_t.cc
tests/test_shmem_recv.cc
tests/test_daqd_thread.cc
+ tests/test_daqd_channel_hashes.cc
daqd_thread.cc
)
target_include_directories(test_daqd_unit_tests PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ../include)
-target_link_libraries(test_daqd_unit_tests PUBLIC ${CMAKE_THREAD_LIBS_INIT})
+target_link_libraries(test_daqd_unit_tests PUBLIC
+ ldastools::framecpp
+ ${CMAKE_THREAD_LIBS_INIT})
add_test(NAME test_daqd_unit_tests
COMMAND test_daqd_unit_tests
WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}")
diff --git a/src/daqd/channel.hh b/src/daqd/channel.hh
index e9488d4e30e6c7b8f4a003172d3c8708ab9e70c7..5864fa2a5752f815d7505fc59e500556b5cd6360 100644
--- a/src/daqd/channel.hh
+++ b/src/daqd/channel.hh
@@ -3,6 +3,7 @@
#include "channel.h"
#include <string.h>
+#include <cstdlib>
typedef struct
{
@@ -129,6 +130,51 @@ public:
}
};
+template < typename Hash >
+class FrameCpp_hash_adapter
+{
+public:
+ explicit FrameCpp_hash_adapter( Hash& hash ) : hash_{ hash }
+ {
+ }
+ FrameCpp_hash_adapter( const FrameCpp_hash_adapter& ) = default;
+
+ void
+ add( const void* data, std::size_t length )
+ {
+ // why is this not const void* ?
+ hash_.Update( const_cast< void* >( data ), length );
+ }
+
+private:
+ Hash& hash_;
+};
+
+template < typename HashObject >
+void
+hash_channel_v0_broken( HashObject& hash, const channel_t& channel )
+{
+ // this is broken, but is kept (for now) for compatibility
+ // the name and units sum the first sizeof(size_t) bytes of their values
+ hash.add( &( channel.chNum ), sizeof( channel.chNum ) );
+ hash.add( &( channel.seq_num ), sizeof( channel.seq_num ) );
+ size_t name_len = strnlen( channel.name, channel_t::channel_name_max_len );
+ hash.add( channel.name, sizeof( name_len ) );
+ hash.add( &( channel.sample_rate ), sizeof( channel.sample_rate ) );
+ hash.add( &( channel.active ), sizeof( channel.active ) );
+ hash.add( &( channel.trend ), sizeof( channel.trend ) );
+ hash.add( &( channel.group_num ), sizeof( channel.group_num ) );
+ hash.add( &( channel.bps ), sizeof( channel.bps ) );
+ hash.add( &( channel.dcu_id ), sizeof( channel.dcu_id ) );
+ hash.add( &( channel.data_type ), sizeof( channel.data_type ) );
+ hash.add( &( channel.signal_gain ), sizeof( channel.signal_gain ) );
+ hash.add( &( channel.signal_slope ), sizeof( channel.signal_slope ) );
+ hash.add( &( channel.signal_offset ), sizeof( channel.signal_offset ) );
+ size_t unit_len =
+ strnlen( channel.signal_units, channel_t::engr_unit_max_len );
+ hash.add( channel.signal_units, sizeof( unit_len ) );
+}
+
template < typename HashObject >
void
hash_channel( HashObject& hash, const channel_t& channel )
diff --git a/src/daqd/daqd.cc b/src/daqd/daqd.cc
index 34f430f33d49cc077ca1f89e609d58e76ed7aecc..395c4c13ccdf64549ccaf5c5a525a70657f49c4d 100644
--- a/src/daqd/daqd.cc
+++ b/src/daqd/daqd.cc
@@ -501,32 +501,27 @@ daqd_c::update_configuration_number( const char* source_address )
if ( num_channels == 0 )
return;
- FrameCPP::Common::MD5Sum check_sum;
+ FrameCPP::Common::MD5Sum check_sum;
+ FrameCpp_hash_adapter< decltype( check_sum ) > hash_wrapper( check_sum );
channel_t* cur = channels;
channel_t* end = channels + num_channels;
- for ( ; cur < end; ++cur )
- {
-
- check_sum.Update( &( cur->chNum ), sizeof( cur->chNum ) );
- check_sum.Update( &( cur->seq_num ), sizeof( cur->seq_num ) );
- size_t name_len = strnlen( cur->name, channel_t::channel_name_max_len );
- check_sum.Update( cur->name, sizeof( name_len ) );
- check_sum.Update( &( cur->sample_rate ), sizeof( cur->sample_rate ) );
- check_sum.Update( &( cur->active ), sizeof( cur->active ) );
- check_sum.Update( &( cur->trend ), sizeof( cur->trend ) );
- check_sum.Update( &( cur->group_num ), sizeof( cur->group_num ) );
- check_sum.Update( &( cur->bps ), sizeof( cur->bps ) );
- check_sum.Update( &( cur->dcu_id ), sizeof( cur->dcu_id ) );
- check_sum.Update( &( cur->data_type ), sizeof( cur->data_type ) );
- check_sum.Update( &( cur->signal_gain ), sizeof( cur->signal_gain ) );
- check_sum.Update( &( cur->signal_slope ), sizeof( cur->signal_slope ) );
- check_sum.Update( &( cur->signal_offset ),
- sizeof( cur->signal_offset ) );
- size_t unit_len =
- strnlen( cur->signal_units, channel_t::engr_unit_max_len );
- check_sum.Update( cur->signal_units, sizeof( unit_len ) );
+
+ std::function< void( const channel_t& ) > hash_cb{
+ [&hash_wrapper]( const channel_t& channel ) {
+ hash_channel( hash_wrapper, channel );
+ }
+ };
+ auto use_broken = parameters( ).get< int >(
+ "USE_BROKEN_CONFIGURATION_NUMBER_HASH", 0 ) == 1;
+ if ( use_broken )
+ {
+ hash_cb = [&hash_wrapper]( const channel_t& channel ) {
+ hash_channel_v0_broken( hash_wrapper, channel );
+ };
}
+ std::for_each( cur, end, hash_cb );
+
check_sum.Finalize( );
std::ostringstream ss;
diff --git a/src/daqd/tests/test_daqd_channel_hashes.cc b/src/daqd/tests/test_daqd_channel_hashes.cc
new file mode 100644
index 0000000000000000000000000000000000000000..11c6bedf71874a333a724eaa21151ede094271c7
--- /dev/null
+++ b/src/daqd/tests/test_daqd_channel_hashes.cc
@@ -0,0 +1,166 @@
+//
+// Created by jonathan.hanks on 10/2/20.
+//
+
+#include "channel.hh"
+#include <cstring>
+#include <sstream>
+#include <string>
+
+#include "framecpp/Common/MD5SumFilter.hh"
+#include "catch.hpp"
+
+namespace
+{
+ channel_t
+ channel1( )
+ {
+ channel_t ch{};
+
+ ch.chNum = 1000;
+ ch.seq_num = 1;
+ ch.id = nullptr;
+ std::strcpy( ch.name, "CHANNEL1_THE_REAL_DEAL" );
+ ch.sample_rate = 16;
+ ch.active = 1;
+ ch.group_num = 1;
+ ch.bps = 16 * 4;
+ ch.offset = 0;
+ ch.bytes = 4;
+ ch.req_rate = 16;
+ ch.dcu_id = 1;
+ ch.ifoid = 0;
+ ch.tp_node = 1;
+ ch.rm_offset = 0;
+ ch.data_type = daq_data_t::_32bit_float;
+ ch.signal_gain = 1.0;
+ ch.signal_slope = 1.0;
+ ch.signal_offset = 0.0;
+ std::strcpy( ch.signal_units, "SOMETHING" );
+ return ch;
+ }
+
+ channel_t
+ channel2( )
+ {
+ channel_t ch{};
+
+ ch.chNum = 1000;
+ ch.seq_num = 1;
+ ch.id = nullptr;
+ std::strcpy( ch.name, "CHANNEL1_IMPOSTER" );
+ ch.sample_rate = 16;
+ ch.active = 1;
+ ch.group_num = 1;
+ ch.bps = 16 * 4;
+ ch.offset = 0;
+ ch.bytes = 4;
+ ch.req_rate = 16;
+ ch.dcu_id = 1;
+ ch.ifoid = 0;
+ ch.tp_node = 1;
+ ch.rm_offset = 0;
+ ch.data_type = daq_data_t::_32bit_float;
+ ch.signal_gain = 1.0;
+ ch.signal_slope = 1.0;
+ ch.signal_offset = 0.0;
+ std::strcpy( ch.signal_units, "SOMETHING_ELSE" );
+ return ch;
+ }
+
+ channel_t
+ channel3( )
+ {
+ channel_t ch{};
+
+ ch.chNum = 1001;
+ ch.seq_num = 1;
+ ch.id = nullptr;
+ std::strcpy( ch.name, "NOT_LIKE_CHANNEL1_IMPOSTER" );
+ ch.sample_rate = 16;
+ ch.active = 1;
+ ch.group_num = 1;
+ ch.bps = 16 * 4;
+ ch.offset = 0;
+ ch.bytes = 4;
+ ch.req_rate = 16;
+ ch.dcu_id = 1;
+ ch.ifoid = 0;
+ ch.tp_node = 1;
+ ch.rm_offset = 0;
+ ch.data_type = daq_data_t::_32bit_float;
+ ch.signal_gain = 1.0;
+ ch.signal_slope = 1.0;
+ ch.signal_offset = 0.0;
+ std::strcpy( ch.signal_units, "SOMETHING_ELSE_ENTIRELY" );
+ return ch;
+ }
+
+ std::string
+ test_hash_this_channel_v0( const channel_t& ch )
+ {
+ FrameCPP::Common::MD5Sum check_sum;
+ FrameCpp_hash_adapter< decltype( check_sum ) > hash_wrapper(
+ check_sum );
+
+ hash_channel_v0_broken( hash_wrapper, ch );
+
+ check_sum.Finalize( );
+ std::ostringstream os;
+ os << check_sum;
+ return os.str( );
+ }
+
+ std::string
+ test_hash_this_channel_v1( const channel_t& ch )
+ {
+ FrameCPP::Common::MD5Sum check_sum;
+ FrameCpp_hash_adapter< decltype( check_sum ) > hash_wrapper(
+ check_sum );
+
+ hash_channel( hash_wrapper, ch );
+
+ check_sum.Finalize( );
+ std::ostringstream os;
+ os << check_sum;
+ return os.str( );
+ }
+} // namespace
+
+TEST_CASE(
+ "hash_channel_v0_broken has easy collisions, reproduce buggy behavior" )
+{
+
+ auto hash1 = test_hash_this_channel_v0( channel1( ) );
+ auto hash2 = test_hash_this_channel_v0( channel2( ) );
+
+ REQUIRE( hash1 == hash2 );
+
+ REQUIRE( hash1 == "1353dc82bb61dddaf97ffdf2400206d0" );
+}
+
+TEST_CASE( "hash_channel_v0_broken doesn't collide on everything" )
+{
+ auto hash1 = test_hash_this_channel_v0( channel1( ) );
+ auto hash3 = test_hash_this_channel_v0( channel3( ) );
+
+ REQUIRE( hash1 != hash3 );
+
+ REQUIRE( hash3 == "c45a8cc2d02de142d87c89d260e7f6d5" );
+}
+
+TEST_CASE( "hash_channel does not have the same collision issue as "
+ "hash_channel_v0_broken" )
+{
+ auto hash1 = test_hash_this_channel_v1( channel1( ) );
+ auto hash2 = test_hash_this_channel_v1( channel2( ) );
+ auto hash3 = test_hash_this_channel_v1( channel3( ) );
+
+ REQUIRE( hash1 != hash2 );
+ REQUIRE( hash1 != hash3 );
+ REQUIRE( hash2 != hash3 );
+
+ REQUIRE( hash1 == "5bd1825ca5aaf1f1354ac3d39fe9a5e3" );
+ REQUIRE( hash2 == "7aa4d0506697a4d7ad7d5268a8a38389" );
+ REQUIRE( hash3 == "6b194dd0ec444ab919072d94ed95c130" );
+}