changed logic of icmp package read/write and operator>> / operator<<

* read and write are implemented on their own, >> and << call them
  (not the other way round as before)
* read returns not only a bool but a more informative flag
* moved operator>> and << from Icmpv4/6packet to base class IcmpPacket
  since they are identical

diff --git a/src/icmp/icmppacket.cpp b/src/icmp/icmppacket.cpp
index 43d272a..50a616d 100644 (file)
--- a/src/icmp/icmppacket.cpp
+++ b/src/icmp/icmppacket.cpp
@@ -19,6 +19,7 @@
  */
 
 #include "icmp/icmppacket.h"
+#include <iostream>
 
 //-----------------------------------------------------------------------------
 // IcmpPacket
@@ -37,3 +38,41 @@ IcmpPacket::IcmpPacket()
 IcmpPacket::~IcmpPacket()
 {
 }
+
+/**
+ *
+ *
+ */
+{
+
+
+}
+
+/**
+ * @brief create packet from data in @a is;  calls IcmpPacket::read
+ *
+ * @param is input stream with hopefully sufficient data
+ *
+ * @returns stream with less data; failbit is set if read returns other than ReadReturnCode_OK
+ */
+std::istream& operator>>(
+        std::istream &is,
+        IcmpPacket &packet
+)
+{
+    IcmpPacket::ReadReturnCode return_code = packet.read(is);
+    if ( return_code != IcmpPacket::ReadReturnCode_OK )
+        is.setstate( std::ios::failbit );
+
+    return is;
+}
+
+
+std::ostream& operator<<(
+        std::ostream& os,
+        const IcmpPacket& packet
+)
+{
+    packet.write( os );
+    return os;
+}

diff --git a/src/icmp/icmppacket.h b/src/icmp/icmppacket.h
index 706eadb..e37686e 100644 (file)
--- a/src/icmp/icmppacket.h
+++ b/src/icmp/icmppacket.h
@@ -43,9 +43,26 @@ public:
     ) const = 0;
     virtual void print_destination_unreachable() const = 0;
 
-    virtual bool read( std::istream &is ) = 0;
+    enum ReadReturnCode {
+        ReadReturnCode_OK = 0,
+        ReadReturnCode_FAIL,
+        ReadReturnCode_NOT_ENOUGH_DATA,
+        ReadReturnCode_BAD_STREAM,
+        ReadReturnCode_INVALID_SIZE
+    };
+
+    virtual ReadReturnCode read( std::istream &is ) = 0;
     virtual bool write( std::ostream &os ) const = 0;
 
+    friend std::istream& operator>>(
+            std::istream &is,
+            IcmpPacket &packet
+    );
+    friend std::ostream& operator<<(
+            std::ostream &os,
+            const IcmpPacket &packet
+    );
+
 protected:
     IcmpPacket();
     virtual ~IcmpPacket();

diff --git a/src/icmp/icmpv4packet.cpp b/src/icmp/icmpv4packet.cpp
index b131be9..20433dc 100644 (file)
--- a/src/icmp/icmpv4packet.cpp
+++ b/src/icmp/icmpv4packet.cpp
@@ -213,19 +213,63 @@ void Icmpv4Packet::print_destination_unreachable() const
 }
 
 /**
- * @brief Read the ICMP packet from the @c istream.
+ * @brief Read (part of) the ICMP packet from the input stream @a is
  *
  * @param is The input stream.
  *
- * @return @c true if the read was successful, or @c false if an error occurred.
+ * @return result of the read, currently one of {ok, fail, not enough data}
  */
-bool Icmpv4Packet::read( istream &is )
+IcmpPacket::ReadReturnCode Icmpv4Packet::read( istream &is )
 {
-    is.clear();
+    if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
 
-    is >> *this;
+    // try to read ip header
+    is >> IpHeader;
 
-    return !is.fail();
+    if ( is.eof() )
+        return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA;
+    else if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
+
+    // try to read the icmp header
+    is >> IcmpPayloadHeader;
+
+    if ( is.eof() )
+        return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA;
+    else if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
+
+    // calculate the size of the ICMP data
+    streamsize data_length = static_cast<streamsize>( IpHeader.get_total_length() )
+                           - static_cast<streamsize>( IpHeader.get_header_length() )
+                           - static_cast<streamsize>( IcmpPayloadHeader.get_header_length() );
+
+    // try to read that amount of data
+    if ( data_length < 0 )
+    {
+        GlobalLogger.error() << "Error: invalid size for optional ICMP data: "
+                             << data_length << endl;
+        is.setstate( ios::failbit );
+        return IcmpPacket::ReadReturnCode_INVALID_SIZE;
+    }
+    else if ( data_length > 0 )
+    {
+        size_t options_size = static_cast<size_t>( data_length );
+        scoped_array<uint8_t> scoped_data( new uint8_t[options_size+1] ); // need a 0 after data
+        memset(scoped_data.get(), 0, (options_size+1)*sizeof(uint8_t));
+        char *char_data = reinterpret_cast<char *>( scoped_data.get() );
+
+        (void) is.read( char_data, data_length );    // leave 0 at the end
+        IcmpPayloadData = char_data;  // implicit cast
+    }
+
+    if ( is.eof() )
+        return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA;
+    else if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
+    else
+        return IcmpPacket::ReadReturnCode_OK;
 }
 
 /**
@@ -235,57 +279,12 @@ bool Icmpv4Packet::read( istream &is )
  *
  * @return @c true if the write was successful, or @c false if an error occurred.
  */
-bool Icmpv4Packet::write( ostream &os ) const
+bool Icmpv4Packet::write( std::ostream &os ) const
 {
     os.clear();
 
-    os << *this;
+    os << IcmpPayloadHeader << IcmpPayloadData;
 
     return !os.fail();
 }
 
-istream& operator>>(
-        istream &is,
-        Icmpv4Packet &packet
-)
-{
-    if (is.good())
-        is >> packet.IpHeader;
-    if (is.good())
-        is >> packet.IcmpPayloadHeader;
-    if (is.good())
-    {
-        streamsize data_length = static_cast<streamsize>( packet.IpHeader.get_total_length() )
-                               - static_cast<streamsize>( packet.IpHeader.get_header_length() )
-                               - static_cast<streamsize>( packet.IcmpPayloadHeader.get_header_length() );
-
-        if ( data_length < 0 )
-        {
-            GlobalLogger.error() << "Error: invalid size for optional ICMP data: "
-                                 << data_length << endl;
-            is.setstate( ios::failbit );
-        }
-        else if ( data_length > 0 )
-        {
-            size_t options_size = static_cast<size_t>( data_length );
-            scoped_array<uint8_t> scoped_data( new uint8_t[options_size+1] ); // need a 0 after data
-            memset(scoped_data.get(), 0, (options_size+1)*sizeof(uint8_t));
-            char *char_data = reinterpret_cast<char *>( scoped_data.get() );
-
-            (void) is.read( char_data, data_length );
-            packet.IcmpPayloadData = char_data;
-        }
-    }
-
-    return is;
-}
-
-ostream& operator<<(
-        ostream& os,
-        const Icmpv4Packet& packet
-)
-{
-    os << packet.IcmpPayloadHeader << packet.IcmpPayloadData;
-
-    return os;
-}

diff --git a/src/icmp/icmpv4packet.h b/src/icmp/icmpv4packet.h
index ac1c7f0..1c0d075 100644 (file)
--- a/src/icmp/icmpv4packet.h
+++ b/src/icmp/icmpv4packet.h
@@ -97,17 +97,9 @@ public:
     ) const;
     virtual void print_destination_unreachable() const;
 
-    virtual bool read( std::istream &is );
-    virtual bool write( std::ostream &os ) const;
+    virtual ReadReturnCode read( std::istream &is );
+    bool write( std::ostream &os ) const;
 
-    friend std::istream& operator>>(
-            std::istream &is,
-            Icmpv4Packet &packet
-    );
-    friend std::ostream& operator<<(
-            std::ostream &os,
-            const Icmpv4Packet &packet
-    );
 
 private:
     bool match(

diff --git a/src/icmp/icmpv6packet.cpp b/src/icmp/icmpv6packet.cpp
index 5d22e90..5545add 100644 (file)
--- a/src/icmp/icmpv6packet.cpp
+++ b/src/icmp/icmpv6packet.cpp
@@ -9,6 +9,7 @@
 #include <iostream>
 
 #include <logfunc.hpp>
+#include <boost/scoped_array.hpp>
 
 #include "boost_assert_handler.h"
 
@@ -17,6 +18,7 @@ using boost::asio::ip::address;
 using boost::date_time::time_resolution_traits_adapted64_impl;
 using boost::posix_time::ptime;
 using boost::posix_time::microsec_clock;
+using boost::scoped_array;
 using I2n::Logger::GlobalLogger;
 
 //-----------------------------------------------------------------------------
@@ -224,19 +226,65 @@ void Icmpv6Packet::print_destination_unreachable() const
 }
 
 /**
- * @brief Read the ICMP packet from the @c istream.
+ * @brief Read (part of) the ICMP packet from the input stream @a is
  *
  * @param is The input stream.
  *
- * @return @c true if the read was successful, or @c false if an error occurred.
+ * @return result of the read, currently one of {ok, fail, not enough data}
  */
-bool Icmpv6Packet::read( istream &is )
+IcmpPacket::ReadReturnCode Icmpv6Packet::read( istream &is )
 {
-    is.clear();
+    if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
 
-    is >> *this;
+#ifdef IPV6_DATA_PRESENT_IN_ISTREAM
+    //TODO WHY IPv6 does not come like IPv4????
+    // try to read ip header
+    is >> IpHeader;
+
+    if ( is.eof() )
+        return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA;
+    else if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
+#endif
 
-    return !is.fail();
+    // try to read the icmp header
+    is >> IcmpPayloadHeader;
+
+    if ( is.eof() )
+        return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA;
+    else if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
+
+    // calculate the size of the ICMP data
+    streamsize data_length = static_cast<streamsize>( IpHeader.get_payload_length() )
+                           - static_cast<streamsize>( IcmpPayloadHeader.get_header_length() );
+
+    // try to read that amount of data
+    if ( data_length < 0 )
+    {
+        GlobalLogger.error() << "Error: invalid size for optional ICMP data: "
+                             << data_length << endl;
+        is.setstate( ios::failbit );
+        return IcmpPacket::ReadReturnCode_INVALID_SIZE;
+    }
+    else if ( data_length > 0 )
+    {
+        size_t options_size = static_cast<size_t>( data_length );
+        scoped_array<uint8_t> scoped_data( new uint8_t[options_size+1] ); // need a 0 after data
+        memset(scoped_data.get(), 0, (options_size+1)*sizeof(uint8_t));
+        char *char_data = reinterpret_cast<char *>( scoped_data.get() );
+
+        (void) is.read( char_data, data_length );    // leave 0 at the end
+        IcmpPayloadData = char_data;  // implicit cast
+    }
+
+    if ( is.eof() )
+        return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA;
+    else if ( !is.good() )
+        return IcmpPacket::ReadReturnCode_BAD_STREAM;
+    else
+        return IcmpPacket::ReadReturnCode_OK;
 }
 
 /**
@@ -246,44 +294,12 @@ bool Icmpv6Packet::read( istream &is )
  *
  * @return @c true if the write was successful, or @c false if an error occurred.
  */
-bool Icmpv6Packet::write( ostream &os ) const
+bool Icmpv6Packet::write( std::ostream &os ) const
 {
     os.clear();
 
-    os << *this;
+    os << IcmpPayloadHeader << IcmpPayloadData;
 
     return !os.fail();
 }
 
-istream& operator>>(
-        istream &is,
-        Icmpv6Packet &packet
-)
-{
-#ifdef IPV6_DATA_PRESENT_IN_ISTREAM
-    //TODO WHY IPv6 does not come like IPv4????
-    if (is.good())
-        is >> packet.IpHeader;
-    if (is.good())
-        is >> packet.IcmpPayloadHeader;
-    if (is.good())
-        is >> packet.IcmpPayloadData;
-#else
-    if (is.good())
-        is >> packet.IcmpPayloadHeader;
-    if (is.good())
-        is >> packet.IcmpPayloadData;
-#endif
-
-    return is;
-}
-
-ostream& operator<<(
-        ostream& os,
-        const Icmpv6Packet& packet
-)
-{
-    os << packet.IcmpPayloadHeader << packet.IcmpPayloadData;
-
-    return os;
-}

diff --git a/src/icmp/icmpv6packet.h b/src/icmp/icmpv6packet.h
index 5d0bd69..0947398 100644 (file)
--- a/src/icmp/icmpv6packet.h
+++ b/src/icmp/icmpv6packet.h
@@ -111,17 +111,9 @@ public:
     ) const;
     virtual void print_destination_unreachable() const;
 
-    virtual bool read( std::istream &is );
-    virtual bool write( std::ostream &os ) const;
+    virtual ReadReturnCode read( std::istream &is );
+    bool write( std::ostream &os ) const;
 
-    friend std::istream& operator>>(
-            std::istream &is,
-            Icmpv6Packet &packet
-    );
-    friend std::ostream& operator<<(
-            std::ostream &os,
-            const Icmpv6Packet &packet
-    );
 
 private:
     bool match(