From dec072c090344a6a7b5cad2ab416349007a006b2 Mon Sep 17 00:00:00 2001 From: Erik Frojdh Date: Fri, 8 Mar 2024 15:04:29 +0100 Subject: [PATCH 1/3] added a few warnings and default build type --- CMakeLists.txt | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b6fbd68..56c4b85 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,34 +7,61 @@ project(aare LANGUAGES C CXX ) -add_library(aare_compiler_flags INTERFACE) -target_compile_features(aare_compiler_flags INTERFACE cxx_std_17) - cmake_policy(SET CMP0135 NEW) cmake_policy(SET CMP0079 NEW) include(GNUInstallDirs) include(FetchContent) +#Set default build type if none was specified +include(cmake/helpers.cmake) +default_build_type("Release") option(AARE_USE_WARNINGS "Eable warnings" ON) option(AARE_PYTHON_BINDINGS "Build python bindings" ON) option(AARE_TESTS "Build tests" ON) option(AARE_EXAMPLES "Build examples" ON) -option(AARE_DEBUG "Compile in debug mode" ON) + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) find_package(fmt 6 REQUIRED) -if (AARE_DEBUG) - target_compile_options(aare_compiler_flags INTERFACE -Og -ggdb3 -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC) -else() +add_library(aare_compiler_flags INTERFACE) +target_compile_features(aare_compiler_flags INTERFACE cxx_std_17) + +#TODO! Explicitly setting flags is not cross platform compatible +if(CMAKE_BUILD_TYPE STREQUAL "Release") + message(STATUS "Release build") target_compile_options(aare_compiler_flags INTERFACE -O3) +else() + target_compile_options( + aare_compiler_flags + INTERFACE + -Og + -ggdb3 + -D_GLIBCXX_DEBUG + -D_GLIBCXX_DEBUG_PEDANTIC + ) endif() if(AARE_USE_WARNINGS) - target_compile_options(aare_compiler_flags INTERFACE -Wall -Wextra -pedantic -Wshadow ) + target_compile_options( + aare_compiler_flags + INTERFACE + -Wall + -Wextra + -pedantic + -Wshadow + -Wnon-virtual-dtor + -Woverloaded-virtual + -Wdouble-promotion + -Wformat=2 + -Wredundant-decls + -Wvla + -Wdouble-promotion + -Werror=return-type #important can cause segfault in optimzed builds + ) endif() if(AARE_BUILD_TESTS) From e8f81e618d335f37ad5424c0c7c3ea0be1ba201e Mon Sep 17 00:00:00 2001 From: Erik Frojdh Date: Fri, 8 Mar 2024 15:23:38 +0100 Subject: [PATCH 2/3] prefixed members rows and cols with m_ to avoid -Wshadow --- core/include/aare/Frame.hpp | 42 +++++++++++++++++++------------------ core/src/Frame.cpp | 11 +++++----- python/src/bindings.cpp | 6 +++--- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/core/include/aare/Frame.hpp b/core/include/aare/Frame.hpp index 78918a6..252103a 100644 --- a/core/include/aare/Frame.hpp +++ b/core/include/aare/Frame.hpp @@ -1,35 +1,37 @@ #pragma once -#include -#include -#include -#include -#include #include "aare/defs.hpp" - - +#include +#include +#include +#include +#include /** * @brief Frame class to represent a single frame of data * model class * should be able to work with streams coming from files or network -*/ + */ +template class Frame { + ssize_t m_rows; + ssize_t m_cols; + DataType *m_data; + ssize_t m_bitdepth = sizeof(DataType) * 8; -template class Frame{ - - public: - ssize_t rows; - ssize_t cols; - DataType* data; - ssize_t bitdepth = sizeof(DataType)*8; - - Frame(std::byte* fp, ssize_t rows, ssize_t cols); + public: + Frame(std::byte *fp, ssize_t rows, ssize_t cols); DataType get(int row, int col); - - ~Frame(){ - delete[] data; + ssize_t rows() const{ + return m_rows; + } + ssize_t cols() const{ + return m_cols; + } + ssize_t bitdepth() const{ + return m_bitdepth; } + ~Frame() { delete[] m_data; } }; typedef Frame Frame16; diff --git a/core/src/Frame.cpp b/core/src/Frame.cpp index f035fc4..97a7b81 100644 --- a/core/src/Frame.cpp +++ b/core/src/Frame.cpp @@ -3,20 +3,19 @@ template Frame::Frame(std::byte* bytes, ssize_t rows, ssize_t cols): - rows(rows), cols(cols) { - data = new DataType[rows*cols]; - std::memcpy(data, bytes, rows*cols*sizeof(DataType)); + m_rows(rows), m_cols(cols) { + m_data = new DataType[rows*cols]; + std::memcpy(m_data, bytes, m_rows*m_cols*sizeof(DataType)); } - template DataType Frame::get(int row, int col) { - if (row < 0 || row >= rows || col < 0 || col >= cols) { + if (row < 0 || row >= m_rows || col < 0 || col >= m_cols) { std::cerr << "Invalid row or column index" << std::endl; return 0; } - return data[row*cols + col]; + return m_data[row*m_cols + col]; } diff --git a/python/src/bindings.cpp b/python/src/bindings.cpp index ab861fe..91c880b 100644 --- a/python/src/bindings.cpp +++ b/python/src/bindings.cpp @@ -27,9 +27,9 @@ PYBIND11_MODULE(_aare, m) { py::class_>(m, "_Frame16") .def(py::init()) .def("get", &Frame::get) - .def_readonly("rows", &Frame::rows) - .def_readonly("cols", &Frame::cols) - .def_readonly("bitdepth", &Frame::bitdepth); + .def_property_readonly("rows", &Frame::rows) + .def_property_readonly("cols", &Frame::cols) + .def_property_readonly("bitdepth", &Frame::bitdepth); From b8cd465a120571534ec89d8b48064ee80ef7a4e0 Mon Sep 17 00:00:00 2001 From: Erik Frojdh Date: Fri, 8 Mar 2024 16:13:17 +0100 Subject: [PATCH 3/3] silence warnings --- file_io/include/aare/File.hpp | 1 + file_io/include/aare/FileFactory.hpp | 3 ++- file_io/include/aare/SubFile.hpp | 17 +++++++++-------- file_io/src/JsonFileFactory.cpp | 10 +++++----- file_io/src/SubFile.cpp | 14 +++++++------- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/file_io/include/aare/File.hpp b/file_io/include/aare/File.hpp index 0a4f628..5eaafab 100644 --- a/file_io/include/aare/File.hpp +++ b/file_io/include/aare/File.hpp @@ -17,6 +17,7 @@ class File { private: public: + virtual ~File() = default; std::filesystem::path fname; std::filesystem::path base_path; std::string base_name, ext; diff --git a/file_io/include/aare/FileFactory.hpp b/file_io/include/aare/FileFactory.hpp index d7a8f62..b11bc8c 100644 --- a/file_io/include/aare/FileFactory.hpp +++ b/file_io/include/aare/FileFactory.hpp @@ -6,13 +6,14 @@ class FileFactory{ // Class that will be used to create File objects // follows the factory pattern protected: - std::filesystem::path fpath; + std::filesystem::path m_fpath; public: static FileFactory* get_factory(std::filesystem::path); // virtual int deleteFile() = 0; virtual File* load_file()=0;//TODO: add option to load all file to memory or keep it on disk virtual void parse_metadata(File*)=0; virtual void parse_fname(File*)=0; + virtual ~FileFactory() = default; diff --git a/file_io/include/aare/SubFile.hpp b/file_io/include/aare/SubFile.hpp index 02efa39..c747e9c 100644 --- a/file_io/include/aare/SubFile.hpp +++ b/file_io/include/aare/SubFile.hpp @@ -7,7 +7,12 @@ class SubFile { protected: FILE *fp = nullptr; - uint16_t bitdepth; + uint16_t m_bitdepth; + std::filesystem::path m_fname; + ssize_t m_rows{}; + ssize_t m_cols{}; + ssize_t n_frames{}; + int m_sub_file_index_{}; public: // pointer to a read_impl function. pointer will be set to the appropriate read_impl function in the constructor @@ -22,11 +27,7 @@ class SubFile { size_t get_frame(std::byte *buffer, int frame_number); // TODO: define the inlines as variables and assign them in constructor - inline size_t bytes_per_frame() { return (bitdepth / 8) * rows * cols; } - inline size_t pixels_per_frame() { return rows * cols; } - std::filesystem::path fname; - ssize_t rows{}; - ssize_t cols{}; - ssize_t n_frames{}; - int sub_file_index_{}; + inline size_t bytes_per_frame() { return (m_bitdepth / 8) * m_rows * m_cols; } + inline size_t pixels_per_frame() { return m_rows * m_cols; } + }; diff --git a/file_io/src/JsonFileFactory.cpp b/file_io/src/JsonFileFactory.cpp index cb19c68..41c2d46 100644 --- a/file_io/src/JsonFileFactory.cpp +++ b/file_io/src/JsonFileFactory.cpp @@ -13,7 +13,7 @@ template JsonFileFactory::JsonFileFactory(std::filesystem::path fpath) { if (not is_master_file(fpath)) throw std::runtime_error("Json file is not a master file"); - this->fpath = fpath; + this->m_fpath = fpath; } template @@ -63,7 +63,7 @@ void JsonFileFactory::open_subfiles(File * template File *JsonFileFactory::load_file() { JsonFile *file = new JsonFile(); - file->fname = this->fpath; + file->fname = this->m_fpath; this->parse_fname(file); this->parse_metadata(file); file->find_number_of_subfiles(); @@ -113,9 +113,9 @@ void JsonFileFactory::find_geometry(File template void JsonFileFactory::parse_fname(File *file) { - file->base_path = this->fpath.parent_path(); - file->base_name = this->fpath.stem(); - file->ext = this->fpath.extension(); + file->base_path = this->m_fpath.parent_path(); + file->base_name = this->m_fpath.stem(); + file->ext = this->m_fpath.extension(); auto pos = file->base_name.rfind("_"); file->findex = std::stoi(file->base_name.substr(pos + 1)); diff --git a/file_io/src/SubFile.cpp b/file_io/src/SubFile.cpp index 7b3ac1d..133ce99 100644 --- a/file_io/src/SubFile.cpp +++ b/file_io/src/SubFile.cpp @@ -9,10 +9,10 @@ */ SubFile::SubFile(std::filesystem::path fname, DetectorType detector, ssize_t rows, ssize_t cols, uint16_t bitdepth) { - this->rows = rows; - this->cols = cols; - this->fname = fname; - this->bitdepth = bitdepth; + this->m_rows = rows; + this->m_cols = cols; + this->m_fname = fname; + this->m_bitdepth = bitdepth; fp = fopen(fname.c_str(), "rb"); if (fp == nullptr) { throw std::runtime_error("Could not open file " + fname.string()); @@ -77,12 +77,12 @@ template size_t SubFile::read_impl_flip(std::byte *buffer) { size_t rc = fread(reinterpret_cast(&tmp[0]), this->bytes_per_frame(), 1, this->fp); // copy to place - const size_t start = this->cols * (this->rows - 1) * sizeof(DataType); - const size_t row_size = this->cols * sizeof(DataType); + const size_t start = this->m_cols * (this->m_rows - 1) * sizeof(DataType); + const size_t row_size = this->m_cols * sizeof(DataType); auto dst = buffer + start; auto src = &tmp[0]; - for (int i = 0; i != this->rows; ++i) { + for (int i = 0; i != this->m_rows; ++i) { memcpy(dst, src, row_size); dst -= row_size; src += row_size;