almost compiles, error: undefined reference to vtable for jungfrauLGADStrixelsDataQuadH5

slsDetectorCalibration/dataStructures/HDF5File.cpp

@@ -231,8 +231,17 @@ void HDF5File::CloseResources () {
 	//if (memspace >= 0)      H5Sclose(memspace); // VH: I am getting memory leaks after recompilation
 }
 
-
-int HDF5File::ReadImage (uint16_t** image, int& iFrame) {
+/*
+ * Function takes uint16_t* argument to make explicit that the caller has to handle memory allocation and deallocation.
+ * This is legacy caused by the structure with which the slsDetectorCalibration cluster finder is written.
+ * Best practice for modern C++ would be to rewrite using smart pointers.
+ */
+int HDF5File::ReadImage (uint16_t* image, int& iFrame) {
+  /*
+   * Originially, this function took uint16_t** but this may lead to memory management issues since image gets redirected
+   * to point to current_image, which is owned by HDF5File.
+   * (Usually, this would be good practice and classic C-style.)
+   */
 
 	// no images in this frame
 	if (frame_index_list[frame_offset[Z]] == 0) {
@@ -252,7 +261,7 @@ int HDF5File::ReadImage (uint16_t** image, int& iFrame) {
 	hid_t memspace = H5Screate_simple (RANK, frame_size, NULL);
 
 	// create hyperslab
-	// If I understand correctly, this puts dataspace such that we read the correct frame
+	// This aligns dataspace such that we read the correct frame
 	if (H5Sselect_hyperslab(dataspace, H5S_SELECT_SET, frame_offset, NULL, frame_size, NULL) < 0 ) {
 		cprintf (RED,"Could not create hyperslab for frame count %llu\n", frame_offset[Z]);
 		CloseResources ();
@@ -260,12 +269,12 @@ int HDF5File::ReadImage (uint16_t** image, int& iFrame) {
 	}
 
 	// read dataset into current_image
-	if (H5Dread(dataset, H5T_STD_U16LE, memspace, dataspace, H5P_DEFAULT, current_image) < 0 ) {
+	if (H5Dread(dataset, H5T_STD_U16LE, memspace, dataspace, H5P_DEFAULT, image /*was 'current_image'*/) < 0 ) {
 		cprintf (RED,"Could not read dataset for frame count %llu\n", frame_offset[Z]);
 		CloseResources ();
 		return -99;
 	}
-	*image = current_image;
+	//*image = current_image; //if uint16_t** is passed, HDF5File owns the resource image points to, which is potentially dangerous
 
 	// return frame number and then increment frame count number
 	unsigned int retval = frame_index_list[frame_offset[Z]];

slsDetectorCalibration/dataStructures/HDF5File.h

@@ -71,7 +71,7 @@ public:
 	 * Read an image into current_image
 	 * @returns frame number read,
 	 */
-	int ReadImage (uint16_t** image, int& iFrame);
+	int ReadImage (uint16_t* image, int& iFrame);
 
 	/**
 	 * Print current image in memory

slsDetectorCalibration/dataStructures/jungfrauLGADStrixelsDataQuadH5.h

@@ -248,6 +248,11 @@ class jungfrauLGADStrixelsDataQuadH5 : public slsDetectorData<uint16_t> {
       remap( xmin, xmax, ymin, ymax );
     }
 
+    //The following functions are pure virtual in the base class. But I don't want them to be accessible here!
+    char* readNextFrame( std::ifstream &filebin );
+    int getFrameNumber(char *buff); //Provided via public method readNextFrame
+    int getPacketNumber(char *buff); //Not provided
+
   public:
 
     using header = sls::defs::sls_receiver_header;
@@ -315,77 +320,64 @@ class jungfrauLGADStrixelsDataQuadH5 : public slsDetectorData<uint16_t> {
 
         uint16_t val = getChannel(data, ix, iy) & 0x3fff;
         return val;
-    };
+    };   
+    
 
-    /**
-
-    Returns the frame number for the given dataset. Purely virtual func.
-    \param buff pointer to the dataset
-    \returns frame number
-
-    */
-    /*
-    int getFrameNumber(char *buff) {
-#ifdef ALDO                                      // VH
-        return ((jf_header *)buff)->bunchNumber; // VH
-#endif                                           // VH
-        return ((header *)buff)->detHeader.frameNumber;
-    };
-    */
-
-    /**
-
-       Returns the packet number for the given dataset. purely virtual func
-       \param buff pointer to the dataset
-       \returns packet number number
-
-    */
-    /*
-    int getPacketNumber(char *buff) {
-#ifdef ALDO // VH
-        // uint32_t fakePacketNumber = 1000;
-        // return fakePacketNumber; //VH //TODO: Keep in mind in case of bugs!
-        // //This is definitely bad!
-        return 1000;
-#endif // VH
-        return ((header *)buff)->detHeader.packetNumber;
-    };
-    */
-
-    char* readNextFrame(const HDF5File& hfile) {
+    char* readNextFrame( HDF5File& hfile ) {
         int fn = 0, iframe = 0;
         return readNextFrame(hfile, fn, iframe);
     };
 
-    char* readNextFrame(const HDF5File& hfile, int& fn) {
+    char* readNextFrame( HDF5File& hfile, int& fn ) {
         int iframe = 0;
         return readNextFrame(hfile, fn, iframe);
     };
 
-    char *readNextFrame(const HDF5File& hfile, int& fn, int& iframe) {
-        char *data = new char[dataSize];
-        char *d = readNextFrame(hfile, fn, iframe, data);
-        if (d == NULL) {
-            delete[] data;
-            data = NULL;
-        }
-        return data;
-    };
+    char* readNextFrame( HDF5File& hfile, int& fn, int& iframe ) {
 
-    //framenumber: Framenumber as written in the file (defined by the time of detector power on)
-    //iframe: Counter how often ReadImage has been called, ReadImage return value
-    char* readNextFrame(const HDF5File& hfile, int& framenumber, int& iframe, char* data) {
-
-      if (iframe>=0) {
-	std::cout << "*";
-	iframe = hfile.ReadImage( data, framenumber )
-	  return data;
+      // Ensure dataSize is a valid size for allocation
+      if (dataSize <= 0) {
+        // Handle error case appropriately, e.g., log an error message
+        return nullptr;
       }
-      std::cout << "#";
-    
-      return NULL;
+
+      char* data = new char[dataSize];
+      char* readResult = readNextFrame(hfile, fn, iframe, data);
+      
+      // Check if reading failed
+      if (readResult == nullptr) {
+        delete[] data;  // Free allocated memory
+        data = nullptr;  // Set to nullptr to avoid dangling pointer
+      }
+      
+      return data; //returning data is equivalent to returning reinterpret_cast<char*>(data_ptr) as they both point to the same memory
+      
     };
 
+
+    /*
+     * This is the most recent function. This is used in the cluster finder!
+     * The overloads are legacy!
+     * Note that caller has to allocate and deallocate memory for data!
+     */
+    char* readNextFrame( HDF5File& hfile, int& framenumber, int& iframe, char* data ) {
+
+      if (iframe >= 0) {
+        std::cout << "*";
+
+	//Storing the reinterpret_cast in the variable data_ptr ensures that I can pass it to a function that expects at uint16_t*
+	uint16_t* data_ptr = reinterpret_cast<uint16_t*>(data); //now data_ptr points where data points (thus modifies the same memory)
+
+        iframe = hfile.ReadImage( data_ptr, framenumber );
+        return data; // return reinterpret_cast<char*>(data_ptr); // Equivalent
+      }
+
+      std::cout << "#";
+      return nullptr;
+    };
+
+ 
+
     /*    Loops over a memory slot until a complete frame is found (i.e. all */
     /*    packets 0 to nPackets, same frame number). purely virtual func \param
      */