From e75a59e7a29764dea966deebf8402a0a2fc7637f Mon Sep 17 00:00:00 2001 From: Christian Herdtweck Date: Wed, 1 Apr 2015 09:47:50 +0200 Subject: [PATCH] moved try-catch around icmp packet parsing from IcmpPacketDistributor to IcmpPacketFactory so can dump when exception is thrown --- src/icmp/icmppacketfactory.cpp | 49 +++++++++++++++++++++-------- src/icmp/icmppacketfactory.h | 11 ------ src/icmp/icmppinger.cpp | 67 ++++++++++++++++------------------------ src/main.cpp | 6 +++- 4 files changed, 68 insertions(+), 65 deletions(-) diff --git a/src/icmp/icmppacketfactory.cpp b/src/icmp/icmppacketfactory.cpp index bcfa74b..03e14dc 100644 --- a/src/icmp/icmppacketfactory.cpp +++ b/src/icmp/icmppacketfactory.cpp @@ -115,21 +115,38 @@ IcmpPacketItem IcmpPacketFactory::create_icmp_packet( // create buffer for saving data in it stringbuf data_backup; + bool have_backup = false; - // read packet from stream, possibly copying data first - if (dump_mode > 0) + // create backup and parse data + try { - // read all data into backup - ostream backup_filler(&data_backup); - backup_filler << is.rdbuf(); - - // create a new stream from backup buffer - // and use that in read - istream is_2(&data_backup); - return_code = icmp_packet->read( is_2 ); + // read packet from stream, possibly copying data first + if (dump_mode > 0) + { + // read all data into backup + ostream backup_filler(&data_backup); + backup_filler << is.rdbuf(); + + // create a new stream from backup buffer + // and use that in read + istream is_2(&data_backup); + have_backup = true; + return_code = icmp_packet->read( is_2 ); + } + else + return_code = icmp_packet->read( is ); + } + catch ( const std::exception &ex ) + { + GlobalLogger.notice() << "Exception during ICMP parse: " << ex.what() + << std::endl; + icmp_packet.reset(); + } + catch ( ... ) + { + GlobalLogger.notice() << "Exception during ICMP parse." << std::endl; + icmp_packet.reset(); } - else - return_code = icmp_packet->read( is ); if ( return_code != IcmpPacket::ReadReturnCode_OK ) { @@ -147,7 +164,13 @@ IcmpPacketItem IcmpPacketFactory::create_icmp_packet( // dump data if had trouble with packet or dumping is set to always if ( dump_mode == 2 || ( dump_mode==1 && !icmp_packet ) ) - dump_packet(data_backup.str()); + { + if ( have_backup ) + dump_packet(data_backup.str()); + else + GlobalLogger.warning() << "Would like to dump packet but " + << "exception occured before backup was created!"; + } if (icmp_packet) GlobalLogger.debug() << "Read packet " << icmp_packet->to_string() diff --git a/src/icmp/icmppacketfactory.h b/src/icmp/icmppacketfactory.h index dd1a07b..3c481f7 100644 --- a/src/icmp/icmppacketfactory.h +++ b/src/icmp/icmppacketfactory.h @@ -60,17 +60,6 @@ public: const uint16_t identifier, const uint16_t sequence_number ); - -private: - static IcmpPacketItem create_icmpv4_packet_echo_request( - const uint16_t identifier, - const uint16_t sequence_number - ); - static IcmpPacketItem create_icmpv6_packet_echo_request( - const uint16_t identifier, - const uint16_t sequence_number - ); - }; #endif // ICMP_PACKET_FACTORY_H diff --git a/src/icmp/icmppinger.cpp b/src/icmp/icmppinger.cpp index f733702..8296252 100644 --- a/src/icmp/icmppinger.cpp +++ b/src/icmp/icmppinger.cpp @@ -516,51 +516,38 @@ void IcmpPacketDistributor::handle_receive( GlobalLogger.info() << "received packet in distributor" << std::endl; - try + std::istream is( &ReplyBuffer ); + if ( !is ) { - std::istream is( &ReplyBuffer ); - if ( !is ) - { - GlobalLogger.error() << "Can't handle ReplyBuffer" << std::endl; - return; - } - - // Decode the reply packet. - IcmpPacketItem icmp_packet = IcmpPacketFactory::create_icmp_packet( - Protocol, is ); - if ( !icmp_packet ) - { - GlobalLogger.warning() << "Ignoring broken ICMP packet" - << std::endl; - } - else - { - GlobalLogger.debug() << "Succesfully parsed ICMP packet" - << std::endl; - - // check which pinger wants this packet - bool packet_matches = false; - BOOST_FOREACH( const IcmpPingerItem &pinger, PingerList ) - { - packet_matches = pinger->handle_receive_icmp_packet( - icmp_packet, bytes_transferred); - if (packet_matches) - break; - } - if (!packet_matches) - GlobalLogger.info() << "Packet did not match any pinger" - << std::endl; - } - + GlobalLogger.error() << "Can't handle ReplyBuffer" << std::endl; + return; } - catch ( const std::exception &ex ) + + // Decode the reply packet. + IcmpPacketItem icmp_packet = IcmpPacketFactory::create_icmp_packet( + Protocol, is ); + if ( !icmp_packet ) { - GlobalLogger.notice() << "Exception during ICMP parse: " << ex.what() - << std::endl; + GlobalLogger.warning() << "Ignoring broken ICMP packet" + << std::endl; } - catch ( ... ) + else { - GlobalLogger.notice() << "Exception during ICMP parse." << std::endl; + GlobalLogger.debug() << "Succesfully parsed ICMP packet" + << std::endl; + + // check which pinger wants this packet + bool packet_matches = false; + BOOST_FOREACH( const IcmpPingerItem &pinger, PingerList ) + { + packet_matches = pinger->handle_receive_icmp_packet( + icmp_packet, bytes_transferred); + if (packet_matches) + break; + } + if (!packet_matches) + GlobalLogger.info() << "Packet did not match any pinger" + << std::endl; } // re-register receive handler diff --git a/src/main.cpp b/src/main.cpp index 53b1b97..8385b44 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -547,7 +547,11 @@ int main( int argc, const char *argv[] ) signal_data.stopped = true; break; } - GlobalLogger.info() << "continuing io_service main loop" << endl; + else if ( signal_data.stopped ) + GlobalLogger.info() << "exiting io_service main loop" << endl; + // ( io_service->stop() has been called already in signal handler) + else + GlobalLogger.info() << "continuing io_service main loop" << endl; } } -- 1.7.1