From e0a99ac48339836eee7589b827c93b6153343268 Mon Sep 17 00:00:00 2001
From: Christian Herdtweck <christian.herdtweck@intra2net.com>
Date: Wed, 11 Feb 2015 16:59:22 +0100
Subject: [PATCH] fix bug that caused crash for broken packets; change order of vars to avoid compiler warnings; minor change in debug output

---
 src/icmp/icmppinger.cpp |   48 +++++++++++++++++++++++++++-------------------
 src/icmp/icmppinger.h   |    6 ++--
 2 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/src/icmp/icmppinger.cpp b/src/icmp/icmppinger.cpp
index 8afee66..703615b 100644
--- a/src/icmp/icmppinger.cpp
+++ b/src/icmp/icmppinger.cpp
@@ -86,6 +86,7 @@ IcmpPinger::IcmpPinger(
         const int echo_reply_timeout_in_sec,
         const IcmpPacketDistributorItem distributor
 ) :
+    PacketDistributor( distributor ),
     DestinationEndpoint(),
     Protocol( protocol ),
     IcmpPacketReceiveTimer( *io_serv ),
@@ -95,8 +96,7 @@ IcmpPinger::IcmpPinger(
     ReplyReceived( false ),
     EchoReplyTimeoutInSec( echo_reply_timeout_in_sec ),
     PingerStatus( PingStatus_NotSent ),
-    PingDoneCallback(),
-    PacketDistributor( distributor )
+    PingDoneCallback()
 {
     // Create "unique" identifier
     boost::uuids::random_generator random_gen;
@@ -219,6 +219,7 @@ bool IcmpPinger::send_echo_request( const IcmpPacketItem icmp_packet )
                    << ": fail sending ping data. " << ex.what() << endl;
     }
 
+    ReplyReceived = false;
     schedule_timeout_echo_reply();
 
     return (bytes_sent > 0);
@@ -227,7 +228,6 @@ bool IcmpPinger::send_echo_request( const IcmpPacketItem icmp_packet )
 void IcmpPinger::schedule_timeout_echo_reply()
 {
     // Wait up to N seconds for a reply.
-    ReplyReceived = false;
     (void) IcmpPacketReceiveTimer.expires_at(
             TimeSent + seconds( EchoReplyTimeoutInSec )
     );
@@ -283,17 +283,24 @@ void IcmpPinger::handle_timeout(const boost::system::error_code& error)
 bool IcmpPinger::handle_receive_icmp_packet(const IcmpPacketItem icmp_packet,
                                             const size_t bytes_transferred )
 {
+    bool does_match = false;
+
     if ( ReplyReceived )
+    {
         // continue, might be an old packet
         // or return false right away, do not want packet anyway...
-        return false;
+        GlobalLogger.info()
+           << DestinationEndpoint.address().to_string()
+           << ": Not interested in packets since we already got a reply"
+           << endl;
+        return does_match;
+    }
 
     // We can receive all ICMP packets received by the host, so we need to
     // filter out only the echo replies that match our identifier,
     // expected sequence number, and destination host address (receive just
     // the ICMP packets from the host we had ping).
 
-    bool does_match = false;
     if ( icmp_packet->match_echo_reply(
                             Identifier, SequenceNumber,
                             DestinationEndpoint.address() ) )
@@ -309,7 +316,7 @@ bool IcmpPinger::handle_receive_icmp_packet(const IcmpPacketItem icmp_packet,
 
         set_ping_status( PingStatus_SuccessReply );
 
-        IcmpPacketReceiveTimer.cancel();                                        //lint !e534
+        IcmpPacketReceiveTimer.cancel();                            //lint !e534
     }
     else if ( icmp_packet->match_destination_unreachable(
                                  Identifier, SequenceNumber,
@@ -326,9 +333,8 @@ bool IcmpPinger::handle_receive_icmp_packet(const IcmpPacketItem icmp_packet,
 
         set_ping_status( PingStatus_FailureDestinationUnreachable );
 
-        IcmpPacketReceiveTimer.cancel();                                        //lint !e534
+        IcmpPacketReceiveTimer.cancel();                            //lint !e534
     }
-    // Unknown ICMP reply, start another receive till timeout
     else
     {
         GlobalLogger.info()
@@ -429,10 +435,10 @@ IcmpPacketDistributor::IcmpPacketDistributor(
             const std::string &network_interface,
             const IoServiceItem io_serv ):
     Protocol( protocol ),
+    Socket( new icmp::socket(*io_serv, protocol) ),
     ReplyBuffer(),
     PingerList()
 {
-    Socket = SocketItem( new icmp::socket(*io_serv, protocol) );
     NetworkInterface<icmp::socket, boost::asio::ip::icmp>
                   NetInterface( network_interface, *Socket );
 
@@ -496,20 +502,22 @@ void IcmpPacketDistributor::handle_receive(
             GlobalLogger.warning() << "Ignoring broken ICMP packet"
                                    << std::endl;
         }
-
-        // check which pinger wants this packet
-        bool packet_matches = false;
-        BOOST_FOREACH( IcmpPingerItem pinger, PingerList )
+        else
         {
-            packet_matches |= pinger->handle_receive_icmp_packet(icmp_packet,
-                                                            bytes_transferred);
-            if (packet_matches)
-                break;
+            // check which pinger wants this packet
+            bool packet_matches = false;
+            BOOST_FOREACH( IcmpPingerItem pinger, PingerList )
+            {
+                packet_matches |= pinger->handle_receive_icmp_packet(
+                                                icmp_packet, bytes_transferred);
+                if (packet_matches)
+                    break;
+            }
+            if (!packet_matches)
+                GlobalLogger.warning() << "Packet did not match any pinger"
+                                       << std::endl;
         }
 
-        if (!packet_matches)
-            GlobalLogger.warning() << "Packet did not match any pinger"
-                                   << std::endl;
     }
     catch ( ... )
     {
diff --git a/src/icmp/icmppinger.h b/src/icmp/icmppinger.h
index ee20c50..85e61ea 100644
--- a/src/icmp/icmppinger.h
+++ b/src/icmp/icmppinger.h
@@ -77,12 +77,12 @@ private:
     void clean_up();
 
 private:
-    /// The socket object
-    SocketItem Socket;
-
     /// Network layer protocol used to ping, IPv4 or IPv6
     icmp::socket::protocol_type Protocol;
 
+    /// The socket object
+    SocketItem Socket;
+
     /// The buffer where the data received will be placed
     boost::asio::streambuf ReplyBuffer;
 
-- 
1.7.1