gh-737: Return status code instead of bool in IParser

roc-streaming · Jan 16, 2025 · 39eb7c8 · 39eb7c8
1 parent 264c175
commit 39eb7c8
Show file tree

Hide file tree

Showing 13 changed files with 33 additions and 26 deletions.
diff --git a/src/internal_modules/roc_fec/block_reader.cpp b/src/internal_modules/roc_fec/block_reader.cpp
@@ -314,7 +314,7 @@ status::StatusCode BlockReader::parse_repaired_packet_(const core::Slice<uint8_t
         return status::StatusNoMem;
     }
 
-    if (!parser_.parse(*pp, buffer)) {
+    if (parser_.parse(*pp, buffer) != status::StatusOK) {
         roc_log(LogDebug, "fec block reader: can't parse repaired packet");
         // Upper code expects StatusBadPacket in this case.
         return status::StatusBadPacket;

diff --git a/src/internal_modules/roc_fec/parser.h b/src/internal_modules/roc_fec/parser.h
@@ -39,11 +39,12 @@ class Parser : public packet::IParser, public core::NonCopyable<> {
     }
 
     //! Parse packet from buffer.
-    virtual bool parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
+    virtual status::StatusCode parse(packet::Packet& packet,
+                                     const core::Slice<uint8_t>& buffer) {
         if (buffer.size() < sizeof(PayloadID)) {
             roc_log(LogDebug, "fec parser: bad packet, size < %d (payload id)",
                     (int)sizeof(PayloadID));
-            return false;
+            return status::StatusBadBuffer;
         }
 
         const PayloadID* payload_id;
@@ -78,7 +79,7 @@ class Parser : public packet::IParser, public core::NonCopyable<> {
             return inner_parser_->parse(packet, fec.payload);
         }
 
-        return true;
+        return status::StatusOK;
     }
 
 private:

diff --git a/src/internal_modules/roc_packet/iparser.h b/src/internal_modules/roc_packet/iparser.h
@@ -40,8 +40,10 @@ class IParser : public core::ArenaAllocation {
     //!  Parses input @p buffer and fills @p packet. If the packet payload contains
     //!  an inner packet, calls the inner parser as well.
     //! @returns
-    //!  true if the packet was successfully parsed or false if the packet is invalid.
-    virtual bool parse(Packet& packet, const core::Slice<uint8_t>& buffer) = 0;
+    //!  status::StatusOK if the packet was successfully parsed,
+    //!  or error code otherwise.
+    virtual status::StatusCode parse(Packet& packet,
+                                     const core::Slice<uint8_t>& buffer) = 0;
 };
 
 } // namespace packet

diff --git a/src/internal_modules/roc_pipeline/receiver_endpoint.cpp b/src/internal_modules/roc_pipeline/receiver_endpoint.cpp
@@ -222,7 +222,7 @@ status::StatusCode ReceiverEndpoint::pull_packets(core::nanoseconds_t current_ti
 
 status::StatusCode ReceiverEndpoint::handle_packet_(const packet::PacketPtr& packet,
                                                     core::nanoseconds_t current_time) {
-    if (!parser_->parse(*packet, packet->buffer())) {
+    if (parser_->parse(*packet, packet->buffer()) != status::StatusOK) {
         roc_log(LogDebug, "receiver endpoint: dropping bad packet: can't parse");
         return status::StatusOK;
     }

diff --git a/src/internal_modules/roc_pipeline/sender_endpoint.cpp b/src/internal_modules/roc_pipeline/sender_endpoint.cpp
@@ -210,7 +210,7 @@ status::StatusCode SenderEndpoint::pull_packets(core::nanoseconds_t current_time
 
 status::StatusCode SenderEndpoint::handle_packet_(const packet::PacketPtr& packet,
                                                   core::nanoseconds_t current_time) {
-    if (!parser_->parse(*packet, packet->buffer())) {
+    if (parser_->parse(*packet, packet->buffer()) != status::StatusOK) {
         roc_log(LogDebug, "sender endpoint: dropping bad packet: can't parse");
         return status::StatusOK;
     }

diff --git a/src/internal_modules/roc_rtcp/parser.cpp b/src/internal_modules/roc_rtcp/parser.cpp
@@ -20,7 +20,8 @@ status::StatusCode Parser::init_status() const {
     return status::StatusOK;
 }
 
-bool Parser::parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
+status::StatusCode Parser::parse(packet::Packet& packet,
+                                 const core::Slice<uint8_t>& buffer) {
     if (!buffer) {
         roc_panic("rtcp parser: buffer is null");
     }
@@ -30,7 +31,7 @@ bool Parser::parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
 
     packet.rtcp()->payload = buffer;
 
-    return true;
+    return status::StatusOK;
 }
 
 } // namespace rtcp

diff --git a/src/internal_modules/roc_rtcp/parser.h b/src/internal_modules/roc_rtcp/parser.h
@@ -33,7 +33,8 @@ class Parser : public packet::IParser, public core::NonCopyable<> {
     virtual status::StatusCode init_status() const;
 
     //! Parse packet from buffer.
-    virtual bool parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer);
+    virtual status::StatusCode parse(packet::Packet& packet,
+                                     const core::Slice<uint8_t>& buffer);
 };
 
 } // namespace rtcp

diff --git a/src/internal_modules/roc_rtp/parser.cpp b/src/internal_modules/roc_rtp/parser.cpp
@@ -25,19 +25,20 @@ status::StatusCode Parser::init_status() const {
     return status::StatusOK;
 }
 
-bool Parser::parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
+status::StatusCode Parser::parse(packet::Packet& packet,
+                                 const core::Slice<uint8_t>& buffer) {
     if (buffer.size() < sizeof(Header)) {
         roc_log(LogDebug, "rtp parser: bad packet: size<%d (rtp header)",
                 (int)sizeof(Header));
-        return false;
+        return status::StatusBadBuffer;
     }
 
     const Header& header = *(const Header*)buffer.data();
 
     if (header.version() != V2) {
         roc_log(LogDebug, "rtp parser: bad version: get=%d expected=%d",
                 (int)header.version(), (int)V2);
-        return false;
+        return status::StatusBadPacket;
     }
 
     size_t header_size = header.header_size();
@@ -49,7 +50,7 @@ bool Parser::parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
     if (buffer.size() < header_size) {
         roc_log(LogDebug, "rtp parser: bad packet: size<%d (rtp header + ext header)",
                 (int)header_size);
-        return false;
+        return status::StatusBadBuffer;
     }
 
     if (header.has_extension()) {
@@ -63,7 +64,7 @@ bool Parser::parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
         roc_log(LogDebug,
                 "rtp parser: bad packet: size<%d (rtp header + ext header + ext data)",
                 (int)header_size);
-        return false;
+        return status::StatusBadBuffer;
     }
 
     size_t payload_begin = header_size;
@@ -75,20 +76,20 @@ bool Parser::parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
         if (payload_begin == payload_end) {
             roc_log(LogDebug,
                     "rtp parser: bad packet: empty payload but padding flag is set");
-            return false;
+            return status::StatusBadPacket;
         }
 
         pad_size = buffer.data()[payload_end - 1];
 
         if (pad_size == 0) {
             roc_log(LogDebug, "rtp parser: bad packet: padding size octet is zero");
-            return false;
+            return status::StatusBadPacket;
         }
 
         if (size_t(payload_end - payload_begin) < size_t(pad_size)) {
             roc_log(LogDebug, "rtp parser: bad packet: padding_size>%d (payload size)",
                     (int)(payload_end - payload_begin));
-            return false;
+            return status::StatusBadPacket;
         }
 
         payload_end -= pad_size;
@@ -118,7 +119,7 @@ bool Parser::parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer) {
         return inner_parser_->parse(packet, rtp.payload);
     }
 
-    return true;
+    return status::StatusOK;
 }
 
 } // namespace rtp

diff --git a/src/internal_modules/roc_rtp/parser.h b/src/internal_modules/roc_rtp/parser.h
@@ -37,7 +37,8 @@ class Parser : public packet::IParser, public core::NonCopyable<> {
     virtual status::StatusCode init_status() const;
 
     //! Parse packet from buffer.
-    virtual bool parse(packet::Packet& packet, const core::Slice<uint8_t>& buffer);
+    virtual status::StatusCode parse(packet::Packet& packet,
+                                     const core::Slice<uint8_t>& buffer);
 
 private:
     const EncodingMap& encoding_map_;

diff --git a/src/tests/roc_fec/test_composer_parser.cpp b/src/tests/roc_fec/test_composer_parser.cpp
@@ -197,7 +197,7 @@ void test_parse(const PacketTest& test) {
 
     packet->set_buffer(buffer);
 
-    CHECK(test.parser->parse(*packet, packet->buffer()));
+    CHECK(test.parser->parse(*packet, packet->buffer()) == status::StatusOK);
 
     check_packet(*packet, test.scheme, test.block_length, test.is_rtp);
 }
@@ -220,7 +220,7 @@ void test_compose_parse(const PacketTest& test) {
     packet::PacketPtr packet2 = packet_factory.new_packet();
     CHECK(packet2);
 
-    CHECK(test.parser->parse(*packet2, packet1->buffer()));
+    CHECK(test.parser->parse(*packet2, packet1->buffer()) == status::StatusOK);
 
     check_packet(*packet2, test.scheme, test.block_length, test.is_rtp);
 }

diff --git a/src/tests/roc_fec/test_helpers/packet_dispatcher.h b/src/tests/roc_fec/test_helpers/packet_dispatcher.h
@@ -212,7 +212,7 @@ class PacketDispatcher : public packet::IWriter {
             FAIL("can't allocate packet");
         }
 
-        if (!parser.parse(*pp, old_pp->buffer())) {
+        if (parser.parse(*pp, old_pp->buffer()) != status::StatusOK) {
             FAIL("can't parse packet");
         }
 

diff --git a/src/tests/roc_pipeline/test_helpers/packet_reader.h b/src/tests/roc_pipeline/test_helpers/packet_reader.h
@@ -143,7 +143,7 @@ class PacketReader : public core::NonCopyable<> {
         packet::PacketPtr pp = packet_factory_.new_packet();
         CHECK(pp);
 
-        CHECK(parser_->parse(*pp, bp));
+        CHECK(parser_->parse(*pp, bp) == status::StatusOK);
         CHECK(pp->flags() & packet::Packet::FlagRTP);
 
         if (first_) {

diff --git a/src/tests/roc_rtp/test_packet_formats.cpp b/src/tests/roc_rtp/test_packet_formats.cpp
@@ -166,7 +166,7 @@ void check_parse_decode(const test::PacketInfo& pi) {
     packet->set_buffer(buffer);
 
     Parser parser(NULL, encoding_map, arena);
-    CHECK(parser.parse(*packet, packet->buffer()));
+    CHECK(parser.parse(*packet, packet->buffer()) == status::StatusOK);
 
     const Encoding* encoding = encoding_map.find_by_pt(packet->rtp()->payload_type);
     CHECK(encoding);