Streamlined byte extraction by removing get()/advance() calls.

We also replace extract_until() with extract(), as the latter is more standard.
There is no need to call advance() before extract(), which is simpler to use.

Also, directly extract file headers into string buffers for simplicity.

Author: LTLA

include/rds2cpp/parse_rda.hpp

@@ -59,35 +59,16 @@ RdaFile parse_rda(Reader_& reader, const ParseRdaOptions& options) {
 
     RdaFile output;
 
-    // Reading the header first. This is the first and only time that 
-    // we need to do a src.valid() check, as we're using the current 
-    // position of the source; in all other cases, it can be assumed
-    // that the source needs to be advance()'d before get().
     {
         try {
-            if (!src.valid()) {
-                throw empty_error();
-            }
-
-            std::string header;
-            header += as_char(src.get());
-            for (int i = 0; i < 4; ++i) {
-                if (!src.advance()) {
-                    throw empty_error();
-                }
-                header += as_char(src.get());
-            }
+            std::string header(5, '\0');
+            quick_extract(src, 5, reinterpret_cast<unsigned char*>(header.data()));
             if (header != "RDX2\n" && header != "RDX3\n") {
-                throw std::runtime_error("unsupported format are currently supported");
+                throw std::runtime_error("only RDX2 or RDX3 formats are currently supported");
             }
 
-            header.clear();
-            for (int i = 0; i < 2; ++i) {
-                if (!src.advance()) {
-                    throw empty_error();
-                }
-                header += as_char(src.get());
-            }
+            header.resize(2);
+            quick_extract(src, 2, reinterpret_cast<unsigned char*>(header.data()));
             if (header != "X\n") {
                 throw std::runtime_error("only RDA files in XDR format are currently supported");
             }

include/rds2cpp/parse_rds.hpp

@@ -59,23 +59,10 @@ RdsFile parse_rds(Reader_& reader, const ParseRdsOptions& options) {
 
     RdsFile output;
 
-    // Reading the header first. This is the first and only time that 
-    // we need to do a src.valid() check, as we're using the current 
-    // position of the source; in all other cases, it can be assumed
-    // that the source needs to be advance()'d before get().
     {
         try {
-            if (!src.valid()) {
-                throw empty_error();
-            }
-
-            std::string header;
-            header += as_char(src.get());
-            if (!src.advance()) {
-                throw empty_error();
-            }
-            header += as_char(src.get());
-
+            std::string header(2, '\0');
+            quick_extract(src, 2, reinterpret_cast<unsigned char*>(header.data()));
             if (header != "X\n") {
                 throw std::runtime_error("only RDS files in XDR format are currently supported");
             }

include/rds2cpp/utils_parse.hpp

@@ -26,19 +26,15 @@ inline void quick_extract(byteme::BufferedReader<unsigned char>& src, std::size_
     if (len == 0) {
         return;
     }
-    if (!src.advance()) {
+    if (!src.valid()) {
         throw empty_error();
     }
-    auto extracted = src.extract_until(len, output);
-    if (extracted < len) {
+    auto extracted = src.extract(len, output);
+    if (extracted.first < len) {
         throw empty_error();
     }
 }
 
-inline char as_char(unsigned char val) {
-    return *reinterpret_cast<const char*>(&val); // make sure we interpret this as a char.
-}
-
 inline bool little_endian() {
     const std::uint32_t value = 1;
     auto lsb = reinterpret_cast<const unsigned char *>(&value);

tests/tests/testthat/test-other.R

@@ -1,26 +1,25 @@
 # library(testthat); library(rds2cpp); source("setup.R"); source("test-other.R")
 
 test_that("length extraction works correctly", {
-    # Remember this advances past the first byte.
-    expect_identical(rds2cpp::parse_length(as.raw(c(0, 0, 0, 0, 16))), 16)
-    expect_identical(rds2cpp::parse_length(as.raw(c(0, 0, 0, 255, 16))), 255 * 256 + 16)
-    expect_identical(rds2cpp::parse_length(as.raw(c(0, 0, 10, 255, 16))), 10 * 256^2 + 255 * 256 + 16)
-    expect_identical(rds2cpp::parse_length(as.raw(c(0, 5, 10, 255, 16))), 5 * 256^3 + 10 * 256^2 + 255 * 256 + 16)
-
-    expect_error(rds2cpp::parse_length(as.raw(c(0, 255, 0, 0, 0)), "should be non-negative"))
-    expect_identical(rds2cpp::parse_length(as.raw(c(0, 255, 255, 255, 255, 0, 0, 0, 1, 0, 0, 0, 1))), 2^32 + 1)
-    expect_identical(rds2cpp::parse_length(as.raw(c(0, 255, 255, 255, 255, 0, 2, 0, 4, 0, 6, 0, 8))), 2 * 256^ 6 + 4 * 256^4 + 6 * 256^2 + 8)
+    expect_identical(rds2cpp::parse_length(as.raw(c(0, 0, 0, 16))), 16)
+    expect_identical(rds2cpp::parse_length(as.raw(c(0, 0, 255, 16))), 255 * 256 + 16)
+    expect_identical(rds2cpp::parse_length(as.raw(c(0, 10, 255, 16))), 10 * 256^2 + 255 * 256 + 16)
+    expect_identical(rds2cpp::parse_length(as.raw(c(5, 10, 255, 16))), 5 * 256^3 + 10 * 256^2 + 255 * 256 + 16)
+
+    expect_error(rds2cpp::parse_length(as.raw(c(255, 0, 0, 0)), "should be non-negative"))
+    expect_identical(rds2cpp::parse_length(as.raw(c(255, 255, 255, 255, 0, 0, 0, 1, 0, 0, 0, 1))), 2^32 + 1)
+    expect_identical(rds2cpp::parse_length(as.raw(c(255, 255, 255, 255, 0, 2, 0, 4, 0, 6, 0, 8))), 2 * 256^ 6 + 4 * 256^4 + 6 * 256^2 + 8)
 })
 
 test_that("string extraction works correctly", {
-    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 0, 2^7, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "UTF-8"))
-    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 0, 2^6, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "latin1"))
-    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 2^2, 0, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "unknown"))
-    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 0, 2^5, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "bytes"))
-
-    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 0, 2^7, 9, 0, 0, 0, 0))), list("", "UTF-8"))
-    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 0, 2^5, 9, 255, 255, 255, 255))), NA_character_)
-    expect_error(rds2cpp::parse_single_string(as.raw(c(0, 0, 0, 2^5, 9, 255, 0, 0, 0))), "non-negative");
+    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 2^7, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "UTF-8"))
+    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 2^6, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "latin1"))
+    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 2^2, 0, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "unknown"))
+    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 2^5, 9, 0, 0, 0, 5, charToRaw('ABCDE')))), list("ABCDE", "bytes"))
+
+    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 2^7, 9, 0, 0, 0, 0))), list("", "UTF-8"))
+    expect_identical(rds2cpp::parse_single_string(as.raw(c(0, 0, 2^5, 9, 255, 255, 255, 255))), NA_character_)
+    expect_error(rds2cpp::parse_single_string(as.raw(c(0, 0, 2^5, 9, 255, 0, 0, 0))), "non-negative");
 })
 
 test_that("RDS preamble extraction works correctly", {