From cee0d71b9c9b65457bf4ab240c0b944aa3b4e689 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Erik=20Fr=C3=B6jdh?= <erik.frojdh@gmail.com>
Date: Thu, 31 Oct 2024 09:43:41 +0100
Subject: [PATCH] added check to prevent segfault on missing subfile

---
 src/Frame.test.cpp   |  8 ++++----
 src/RawFile.cpp      |  9 ++++++---
 src/RawFile.test.cpp | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/src/Frame.test.cpp b/src/Frame.test.cpp
index e8ce5d2..33bbbb6 100644
--- a/src/Frame.test.cpp
+++ b/src/Frame.test.cpp
@@ -19,7 +19,7 @@ TEST_CASE("Construct a frame") {
     // data should be initialized to 0
     for (size_t i = 0; i < rows; i++) {
         for (size_t j = 0; j < cols; j++) {
-            uint8_t *data = (uint8_t *)frame.get(i, j);
+            uint8_t *data = (uint8_t *)frame.pixel_ptr(i, j);
             REQUIRE(data != nullptr);
             REQUIRE(*data == 0);
         }
@@ -40,7 +40,7 @@ TEST_CASE("Set a value in a 8 bit frame") {
     // only the value we did set should be non-zero
     for (size_t i = 0; i < rows; i++) {
         for (size_t j = 0; j < cols; j++) {
-            uint8_t *data = (uint8_t *)frame.get(i, j);
+            uint8_t *data = (uint8_t *)frame.pixel_ptr(i, j);
             REQUIRE(data != nullptr);
             if (i == 5 && j == 7) {
                 REQUIRE(*data == value);
@@ -65,7 +65,7 @@ TEST_CASE("Set a value in a 64 bit frame") {
     // only the value we did set should be non-zero
     for (size_t i = 0; i < rows; i++) {
         for (size_t j = 0; j < cols; j++) {
-            uint64_t *data = (uint64_t *)frame.get(i, j);
+            uint64_t *data = (uint64_t *)frame.pixel_ptr(i, j);
             REQUIRE(data != nullptr);
             if (i == 5 && j == 7) {
                 REQUIRE(*data == value);
@@ -134,7 +134,7 @@ TEST_CASE("test explicit copy constructor") {
     Frame frame(rows, cols, Dtype::from_bitdepth(bitdepth));
     std::byte *data = frame.data();
 
-    Frame frame2 = frame.copy();
+    Frame frame2 = frame.clone();
 
     // state of the original object
     REQUIRE(frame.rows() == rows);
diff --git a/src/RawFile.cpp b/src/RawFile.cpp
index f7ca21c..79cdd02 100644
--- a/src/RawFile.cpp
+++ b/src/RawFile.cpp
@@ -400,10 +400,13 @@ void RawFile::read_into(std::byte *image_buf, size_t n_frames) {
 }
 
 size_t RawFile::frame_number(size_t frame_index) {
-    if (frame_index > this->m_total_frames) {
-        throw std::runtime_error(LOCATION + "Frame number out of range");
+    if (frame_index >= this->m_total_frames) {
+        throw std::runtime_error(LOCATION + " Frame number out of range");
+    }
+    size_t subfile_id = frame_index / this->max_frames_per_file;
+    if(subfile_id >= this->subfiles.size()){
+        throw std::runtime_error(LOCATION + " Subfile out of range. Possible missing data.");
     }
-    size_t const subfile_id = frame_index / this->max_frames_per_file;
     return this->subfiles[subfile_id][0]->frame_number(frame_index % this->max_frames_per_file);
 }
 
diff --git a/src/RawFile.test.cpp b/src/RawFile.test.cpp
index 295747e..dabacad 100644
--- a/src/RawFile.test.cpp
+++ b/src/RawFile.test.cpp
@@ -32,6 +32,41 @@ TEST_CASE("Read frame numbers from a jungfrau raw file") {
     }
 }
 
+TEST_CASE("Read a frame number too high throws") {
+    auto fpath = test_data_path() / "jungfrau" / "jungfrau_single_master_0.json";
+    REQUIRE(std::filesystem::exists(fpath));
+
+    File f(fpath, "r");
+
+    // we know this file has 10 frames with frame numbers 1 to 10
+    // f0 1,2,3
+    // f1 4,5,6
+    // f2 7,8,9
+    // f3 10
+    REQUIRE_THROWS(f.frame_number(10));
+}
+
+TEST_CASE("Read a frame numbers where the subfile is missing throws") {
+    auto fpath = test_data_path() / "jungfrau" / "jungfrau_missing_subfile_master_0.json";
+    REQUIRE(std::filesystem::exists(fpath));
+
+    File f(fpath, "r");
+
+    // we know this file has 10 frames with frame numbers 1 to 10
+    // f0 1,2,3
+    // f1 4,5,6 - but files f1-f3 are missing
+    // f2 7,8,9 - gone  
+    // f3 10    - gone
+    REQUIRE(f.frame_number(0) == 1);
+    REQUIRE(f.frame_number(1) == 2);
+    REQUIRE(f.frame_number(2) == 3);
+    REQUIRE_THROWS(f.frame_number(4));
+    REQUIRE_THROWS(f.frame_number(7));
+    REQUIRE_THROWS(f.frame_number(937));
+    // REQUIRE_THROWS(f.frame_number(10));
+}
+
+
 TEST_CASE("Read data from a jungfrau 500k single port raw file") {
     auto fpath = test_data_path() / "jungfrau" / "jungfrau_single_master_0.json";
     REQUIRE(std::filesystem::exists(fpath));