From 7dca65022b970f7f6df103b6d1a268a5ba04bced Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 15:28:20 +0800 Subject: [PATCH 01/21] major fixes --- src/Rfits.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index fc4929a..ff915c9 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -28,7 +28,8 @@ class fits_file { public: fits_file() {} fits_file(fitsfile *fptr) : m_fptr(fptr) {} - fits_file(const fits_file &other) = default; + fits_file(const fits_file &other) = delete; + fits_file &operator=(const fits_file &other) = delete; fits_file(fits_file &&other) = default; ~fits_file() { @@ -105,7 +106,7 @@ static SEXP ensure_lossless_32bit_int(const std::vector &values) auto doesnt_fit_in_r_int = std::any_of(values.begin(), values.end(), [](long value) { return value > std::numeric_limits::max(); }); if (doesnt_fit_in_r_int) { Rcpp::NumericVector output(values.size()); - std::memcpy(&(output[0]), &(values[0]), values.size() * sizeof(double)); + std::memcpy(&(output[0]), &(values[0]), values.size() * sizeof(long)); output.attr("class") = "integer64"; return output; } @@ -687,7 +688,9 @@ SEXP Cfits_read_header_raw(Rcpp::String filename, int ext=1){ Rcpp::StringVector out(1); - char *header = (char *)malloc(FLEN_CARD * nkeys); + // fits_hdr2str allocates its own buffer and overwrites the pointer; + // do not pre-allocate here or that allocation is leaked. + char *header = nullptr; fits_invoke(hdr2str, fptr, 1, nullptr, 0, &header, &nkeys); @@ -768,7 +771,7 @@ SEXP Cfits_read_img_subset(Rcpp::String filename, int ext=1, int datatype= -32, } } - int nelements = naxis1 * naxis2 * naxis3 * naxis4; + long nelements = (long)naxis1 * naxis2 * naxis3 * naxis4; long inc[] = {sparse, sparse, sparse, sparse}; // Rcpp::Rcout << nelements <<"\n"; @@ -811,7 +814,7 @@ SEXP Cfits_read_img_subset(Rcpp::String filename, int ext=1, int datatype= -32, return ensure_lossless_32bit_int(pixels); }else if (datatype==LONGLONG_IMG){ std::vector pixels(nelements); - fits_invoke(read_subset, fptr, TLONG, fpixel, lpixel, inc, + fits_invoke(read_subset, fptr, TLONGLONG, fpixel, lpixel, inc, &nullvals, pixels.data(), &anynull); Rcpp::NumericVector pixel_matrix(nelements); std::memcpy(&(pixel_matrix[0]), &(pixels[0]), nelements * sizeof(double)); From d6630ef9fcea390ac7cd3e1fe532516462feac61 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 15:35:24 +0800 Subject: [PATCH 02/21] Minor changes --- src/Rfits.cpp | 144 ++++++++++++++++++++++---------------------------- 1 file changed, 63 insertions(+), 81 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index ff915c9..4d6c99a 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -10,7 +10,7 @@ using namespace Rcpp; -std::runtime_error fits_status_to_exception(const char *func_name, int status) +[[noreturn]] void fits_throw_exception(const char *func_name, int status) { char err_msg[30]; fits_get_errstatus(status, err_msg); @@ -70,7 +70,7 @@ void _fits_invoke(const char *func_name, F&& func, Args&& ... args) int status = 0; func(std::forward(args)..., &status); if (status) { - throw fits_status_to_exception(func_name, status); + fits_throw_exception(func_name, status); } } @@ -80,7 +80,7 @@ fitsfile *fits_safe_open_file(const char *filename, int mode) fitsfile *file; fits_open_file(&file, const_cast(filename), mode, &status); if (status) { - throw fits_status_to_exception("open_file", status); + fits_throw_exception("open_file", status); } return file; } @@ -103,7 +103,10 @@ static SEXP ensure_lossless_32bit_int(const std::vector &values) // R's integers are signed, so if any value is >= 2^31 // we return the whole array as a bit64 array // (i.e., a double array with class "integer64"). - auto doesnt_fit_in_r_int = std::any_of(values.begin(), values.end(), [](long value) { return value > std::numeric_limits::max(); }); + auto doesnt_fit_in_r_int = std::any_of(values.begin(), values.end(), [](long value) { + return value > std::numeric_limits::max() || + value < std::numeric_limits::min(); + }); if (doesnt_fit_in_r_int) { Rcpp::NumericVector output(values.size()); std::memcpy(&(output[0]), &(values[0]), values.size() * sizeof(long)); @@ -167,17 +170,14 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, int cwidth; fits_invoke(get_col_display_width, fptr, colref, &cwidth); - char **data = (char **)malloc(sizeof(char *) * nrow); - for (ii = 0 ; ii < nrow ; ii++ ) { - data[ii] = (char*)calloc(cwidth + 1, 1); - } - fits_invoke(read_col, fptr, TSTRING, colref, startrow, 1, nrow, nullptr, data, &anynull); + // Use RAII storage so memory is freed even if fits_invoke throws. + std::vector> storage(nrow, std::vector(cwidth + 1, '\0')); + std::vector data(nrow); + for (int i = 0; i < nrow; i++) data[i] = storage[i].data(); + + fits_invoke(read_col, fptr, TSTRING, colref, startrow, 1, nrow, nullptr, data.data(), &anynull); Rcpp::StringVector out(nrow); - std::copy(data, data + nrow, out.begin()); - for (int i = 0; i != nrow; i++) { - free(data[i]); - } - free(data); + std::copy(data.begin(), data.end(), out.begin()); return out; } else if ( typecode == TBIT ) { @@ -254,16 +254,14 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, long nullval = -999; std::vector col(nrow); fits_invoke(read_col, fptr, TLONG, colref, startrow, 1, nrow, &nullval, col.data(), &anynull); - Rcpp::NumericVector out(nrow); - std::copy(col.begin(), col.end(), out.begin()); - return out; + return ensure_lossless_32bit_int(col); } else if ( typecode == TLONGLONG ) { - long nullval = -999; + int64_t nullval = -999; std::vector col(nrow); fits_invoke(read_col, fptr, TLONGLONG, colref, startrow, 1, nrow, &nullval, col.data(), &anynull); Rcpp::NumericVector out(nrow); - std::memcpy(&(out[0]), &(col[0]), nrow * sizeof(double)); + std::memcpy(&(out[0]), &(col[0]), nrow * sizeof(int64_t)); out.attr("class") = "integer64"; return out; } @@ -368,7 +366,7 @@ void Cfits_create_bintable(Rcpp::String filename, int tfields, // [[Rcpp::export]] void Cfits_write_col(Rcpp::String filename, SEXP data, int nrow, int colref=1, int ext=2, int typecode=1){ - int hdutype,ii; + int hdutype; fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); fits_invoke(movabs_hdu, fptr, ext, &hdutype); @@ -406,23 +404,20 @@ SEXP Cfits_read_key(Rcpp::String filename, Rcpp::String keyname, int typecode, i if ( typecode == TDOUBLE ) { Rcpp::NumericVector out(1); - std::vector keyvalue(1); - fits_invoke(read_key, fptr, TDOUBLE, keyname.get_cstring(), keyvalue.data(), comment); - std::copy(keyvalue.begin(), keyvalue.end(), out.begin()); + fits_invoke(read_key, fptr, TDOUBLE, keyname.get_cstring(), &out[0], comment); return(out); }else if ( typecode == TSTRING){ Rcpp::StringVector out(1); - //std::vector keyvalue(1); char keyvalue[81]; fits_invoke(read_key, fptr, TSTRING, keyname.get_cstring(), keyvalue, comment); out[0] = keyvalue; - //std::copy(keyvalue.begin(), keyvalue.end(), out.begin()); return(out); }else if ( typecode == TLONG ) { + // TLONG maps to C `long`; read into a long buffer to avoid mismatched size. + long keyvalue; + fits_invoke(read_key, fptr, TLONG, keyname.get_cstring(), &keyvalue, comment); Rcpp::IntegerVector out(1); - std::vector keyvalue(1); - fits_invoke(read_key, fptr, TLONG, keyname.get_cstring(), keyvalue.data(), comment); - std::copy(keyvalue.begin(), keyvalue.end(), out.begin()); + out[0] = static_cast(keyvalue); return(out); } throw std::runtime_error("unsupported type"); @@ -528,7 +523,7 @@ void Cfits_create_image(Rcpp::String filename, int naxis, long naxis1=100 , long void Cfits_write_pix(Rcpp::String filename, SEXP data, int ext=1, int datatype= -32, int naxis=2, long naxis1=100 , long naxis2=100, long naxis3=1, long naxis4=1) { - int hdutype, ii; + int hdutype; long nelements = naxis1 * naxis2 * naxis3 * naxis4; long fpixel_vector[] = {1}; @@ -540,40 +535,34 @@ void Cfits_write_pix(Rcpp::String filename, SEXP data, int ext=1, int datatype= fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - //below need to work for integers and doubles: + const int *int_src = INTEGER(data); + const double *dbl_src = REAL(data); + if(datatype == TBYTE){ - // char *data_b = (char *)malloc(nelements * sizeof(char)); std::vector data_b(nelements); - for (ii = 0; ii < nelements; ii++) { - data_b[ii] = INTEGER(data)[ii]; - } + std::transform(int_src, int_src + nelements, data_b.begin(), + [](int v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_b.data()); }else if(datatype == TINT){ - fits_invoke(write_pix, fptr, datatype, fpixel, nelements, INTEGER(data)); + fits_invoke(write_pix, fptr, datatype, fpixel, nelements, int_src); }else if(datatype == TSHORT){ - // short *data_s = (short *)malloc(nelements * sizeof(short)); std::vector data_s(nelements); - for (ii = 0; ii < nelements; ii++) { - data_s[ii] = INTEGER(data)[ii]; - } + std::transform(int_src, int_src + nelements, data_s.begin(), + [](int v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_s.data()); }else if(datatype == TLONG){ - // long *data_l = (long *)malloc(nelements * sizeof(long)); std::vector data_l(nelements); - for (ii = 0; ii < nelements; ii++) { - data_l[ii] = INTEGER(data)[ii]; - } + std::transform(int_src, int_src + nelements, data_l.begin(), + [](int v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_l.data()); }else if(datatype == TLONGLONG){ - fits_invoke(write_pix, fptr, datatype, fpixel, nelements, REAL(data)); + fits_invoke(write_pix, fptr, datatype, fpixel, nelements, dbl_src); }else if(datatype == TDOUBLE){ - fits_invoke(write_pix, fptr, datatype, fpixel, nelements, REAL(data)); + fits_invoke(write_pix, fptr, datatype, fpixel, nelements, dbl_src); }else if(datatype == TFLOAT){ - // float *data_f = (float *)malloc(nelements * sizeof(float)); std::vector data_f(nelements); - for (ii = 0; ii < nelements; ii++) { - data_f[ii] = REAL(data)[ii]; - } + std::transform(dbl_src, dbl_src + nelements, data_f.begin(), + [](double v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_f.data()); } } @@ -787,11 +776,9 @@ SEXP Cfits_read_img_subset(Rcpp::String filename, int ext=1, int datatype= -32, std::copy(pixels.begin(), pixels.end(), pixel_matrix.begin()); return(pixel_matrix); }else if (datatype==DOUBLE_IMG){ - std::vector pixels(nelements); + Rcpp::NumericVector pixel_matrix(Rcpp::no_init(nelements)); fits_invoke(read_subset, fptr, TDOUBLE, fpixel, lpixel, inc, - &nullvals, pixels.data(), &anynull); - Rcpp::NumericVector pixel_matrix(nelements); - std::copy(pixels.begin(), pixels.end(), pixel_matrix.begin()); + &nullvals, pixel_matrix.begin(), &anynull); return(pixel_matrix); }else if (datatype==BYTE_IMG){ std::vector pixels(nelements); @@ -830,7 +817,8 @@ void Cfits_write_img_subset(Rcpp::String filename, SEXP data, int ext=1, int dat long lpixel0=100, long lpixel1=100, long lpixel2=1, long lpixel3=1 ){ - int hdutype, ii, nelements; + int hdutype; + long nelements; // Rcpp::Rcout << filename.get_cstring() <<"\n"; // Rcpp::Rcout << datatype <<"\n"; @@ -867,57 +855,51 @@ void Cfits_write_img_subset(Rcpp::String filename, SEXP data, int ext=1, int dat int naxis4 = (lpixel[3] - fpixel[3] + 1); if (naxis == 1) { - nelements = naxis1; + nelements = (long)naxis1; } else if (naxis == 2) { - nelements = naxis1 * naxis2; + nelements = (long)naxis1 * naxis2; } else if (naxis == 3) { - nelements = naxis1 * naxis2 * naxis3; + nelements = (long)naxis1 * naxis2 * naxis3; } else if (naxis == 4) { - nelements = naxis1 * naxis2 * naxis3 * naxis4; + nelements = (long)naxis1 * naxis2 * naxis3 * naxis4; } else { Rcpp::stop("naxis=%d doesn't meet condition: 1 <= naxis <= 4", naxis); } // Rcpp::Rcout << nelements <<"\n"; - - //below need to work for integers and doubles: + + const int *int_src = INTEGER(data); + const double *dbl_src = REAL(data); + if(datatype == TBYTE){ - // char *data_b = (char *)malloc(nelements * sizeof(char)); std::vector data_b(nelements); - for (ii = 0; ii < nelements; ii++) { - data_b[ii] = INTEGER(data)[ii]; - } + std::transform(int_src, int_src + nelements, data_b.begin(), + [](int v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_b.data()); }else if(datatype == TINT){ - fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, INTEGER(data)); + fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, int_src); }else if(datatype == TSHORT){ - // short *data_s = (short *)malloc(nelements * sizeof(short)); std::vector data_s(nelements); - for (ii = 0; ii < nelements; ii++) { - data_s[ii] = INTEGER(data)[ii]; - } + std::transform(int_src, int_src + nelements, data_s.begin(), + [](int v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_s.data()); }else if(datatype == TLONG){ - // long *data_l = (long *)malloc(nelements * sizeof(long)); std::vector data_l(nelements); - for (ii = 0; ii < nelements; ii++) { - data_l[ii] = INTEGER(data)[ii]; - } + std::transform(int_src, int_src + nelements, data_l.begin(), + [](int v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_l.data()); }else if(datatype == TLONGLONG){ - fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, REAL(data)); + fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, dbl_src); }else if(datatype == TDOUBLE){ - fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, REAL(data)); + fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, dbl_src); }else if(datatype == TFLOAT){ - // float *data_f = (float *)malloc(nelements * sizeof(float)); std::vector data_f(nelements); - for (ii = 0; ii < nelements; ii++) { - data_f[ii] = REAL(data)[ii]; - } + std::transform(dbl_src, dbl_src + nelements, data_f.begin(), + [](double v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_f.data()); } } @@ -931,7 +913,7 @@ void Cfits_write_chksum(Rcpp::String filename){ // [[Rcpp::export]] SEXP Cfits_verify_chksum(Rcpp::String filename, int verbose){ int dataok, hduok; - fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); + fits_file fptr = fits_safe_open_file(filename.get_cstring(), READONLY); fits_invoke(verify_chksum, fptr, &dataok, &hduok); if(verbose == 1){ if(dataok == 1){ @@ -962,7 +944,7 @@ SEXP Cfits_verify_chksum(Rcpp::String filename, int verbose){ // [[Rcpp::export]] SEXP Cfits_get_chksum(Rcpp::String filename){ unsigned long datasum, hdusum; - fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); + fits_file fptr = fits_safe_open_file(filename.get_cstring(), READONLY); fits_invoke(get_chksum, fptr, &datasum, &hdusum); Rcpp::NumericVector out(2); out.attr("class") = "integer64"; From 19092b8b6e9a0786c6050b4c8af285add275059f Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 15:39:58 +0800 Subject: [PATCH 03/21] Some more optimisations --- src/Rfits.cpp | 58 +++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 4d6c99a..8824e3d 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -100,23 +100,24 @@ std::vector to_string_vector(const Rcpp::CharacterVector &strings) static SEXP ensure_lossless_32bit_int(const std::vector &values) { - // R's integers are signed, so if any value is >= 2^31 - // we return the whole array as a bit64 array - // (i.e., a double array with class "integer64"). - auto doesnt_fit_in_r_int = std::any_of(values.begin(), values.end(), [](long value) { - return value > std::numeric_limits::max() || - value < std::numeric_limits::min(); - }); - if (doesnt_fit_in_r_int) { - Rcpp::NumericVector output(values.size()); - std::memcpy(&(output[0]), &(values[0]), values.size() * sizeof(long)); - output.attr("class") = "integer64"; - return output; + // Optimistically fill an IntegerVector in a single pass. + // If any value is out of int32 range, immediately fall back to integer64 + // via memcpy — discarding the partial work but avoiding a second scan + // over the full array in the common case where all values fit. + const size_t n = values.size(); + Rcpp::IntegerVector int_out(Rcpp::no_init(n)); + for (size_t i = 0; i < n; i++) { + long v = values[i]; + if (v > std::numeric_limits::max() || + v < std::numeric_limits::min()) { + Rcpp::NumericVector output(n); + std::memcpy(&(output[0]), &(values[0]), n * sizeof(long)); + output.attr("class") = "integer64"; + return output; + } + int_out[i] = static_cast(v); } - // otherwise they fit into R's signed integer vector - Rcpp::IntegerVector output(values.size()); - std::copy(values.begin(), values.end(), output.begin()); - return output; + return int_out; } // [[Rcpp::export]] @@ -170,10 +171,11 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, int cwidth; fits_invoke(get_col_display_width, fptr, colref, &cwidth); - // Use RAII storage so memory is freed even if fits_invoke throws. - std::vector> storage(nrow, std::vector(cwidth + 1, '\0')); + // Single flat allocation: better cache locality and fewer heap ops than + // vector>. RAII ensures cleanup even if fits_invoke throws. + std::vector storage((long)nrow * (cwidth + 1), '\0'); std::vector data(nrow); - for (int i = 0; i < nrow; i++) data[i] = storage[i].data(); + for (int i = 0; i < nrow; i++) data[i] = storage.data() + i * (cwidth + 1); fits_invoke(read_col, fptr, TSTRING, colref, startrow, 1, nrow, nullptr, data.data(), &anynull); Rcpp::StringVector out(nrow); @@ -206,10 +208,8 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, } else if ( typecode == TINT ) { int nullval = -999; - std::vector col(nrow); - fits_invoke(read_col, fptr, TINT, colref, startrow, 1, nrow, &nullval, col.data(), &anynull); - Rcpp::IntegerVector out(nrow); - std::copy(col.begin(), col.end(), out.begin()); + Rcpp::IntegerVector out(Rcpp::no_init(nrow)); + fits_invoke(read_col, fptr, TINT, colref, startrow, 1, nrow, &nullval, out.begin(), &anynull); return out; } else if ( typecode == TUINT ) { @@ -267,10 +267,8 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, } else if ( typecode == TDOUBLE ) { double nullval = -999; - std::vector col(nrow); - fits_invoke(read_col, fptr, TDOUBLE, colref, startrow, 1, nrow, &nullval, col.data(), &anynull); - Rcpp::NumericVector out(nrow); - std::copy(col.begin(), col.end(), out.begin()); + Rcpp::NumericVector out(Rcpp::no_init(nrow)); + fits_invoke(read_col, fptr, TDOUBLE, colref, startrow, 1, nrow, &nullval, out.begin(), &anynull); return out; } throw std::runtime_error("unsupported type"); @@ -373,8 +371,8 @@ void Cfits_write_col(Rcpp::String filename, SEXP data, int nrow, int colref=1, i if ( typecode == TSTRING ) { std::vector s_data(nrow); - for (ii = 0 ; ii < nrow ; ii++ ) { - s_data[ii] = (char*)CHAR(STRING_ELT(data, ii)); + for (int i = 0; i < nrow; i++) { + s_data[i] = (char*)CHAR(STRING_ELT(data, i)); } fits_invoke(write_col, fptr, typecode, colref, 1, 1, nrow, s_data.data()); }else if (typecode == TBIT){ @@ -804,7 +802,7 @@ SEXP Cfits_read_img_subset(Rcpp::String filename, int ext=1, int datatype= -32, fits_invoke(read_subset, fptr, TLONGLONG, fpixel, lpixel, inc, &nullvals, pixels.data(), &anynull); Rcpp::NumericVector pixel_matrix(nelements); - std::memcpy(&(pixel_matrix[0]), &(pixels[0]), nelements * sizeof(double)); + std::memcpy(&(pixel_matrix[0]), &(pixels[0]), nelements * sizeof(int64_t)); pixel_matrix.attr("class") = "integer64"; return(pixel_matrix); } From 1d0993f688c8ed1dc7b7dc95afc4e8e33a1b9ff9 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 15:49:13 +0800 Subject: [PATCH 04/21] Compiles fine --- src/Rfits.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 8824e3d..2a88e58 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -151,7 +151,7 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, Rcpp::stop("startrow must be ≥ 1"); } - int hdutype,anynull,typecode,ii; + int hdutype,anynull,typecode; long repeat,width,nrow_total; fits_file fptr = fits_safe_open_file(filename.get_cstring(), READONLY); @@ -533,8 +533,8 @@ void Cfits_write_pix(Rcpp::String filename, SEXP data, int ext=1, int datatype= fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - const int *int_src = INTEGER(data); - const double *dbl_src = REAL(data); + int *int_src = INTEGER(data); + double *dbl_src = REAL(data); if(datatype == TBYTE){ std::vector data_b(nelements); @@ -870,8 +870,8 @@ void Cfits_write_img_subset(Rcpp::String filename, SEXP data, int ext=1, int dat // Rcpp::Rcout << nelements <<"\n"; - const int *int_src = INTEGER(data); - const double *dbl_src = REAL(data); + int *int_src = INTEGER(data); + double *dbl_src = REAL(data); if(datatype == TBYTE){ std::vector data_b(nelements); From daf70008ed8cb2851027eaf89e1869e18d44f663 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 15:58:05 +0800 Subject: [PATCH 05/21] Clean up --- src/Rfits.cpp | 175 ++++++++++++++++++++++---------------------------- 1 file changed, 77 insertions(+), 98 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 2a88e58..e74d72a 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -6,8 +6,6 @@ #include "cfitsio/fitsio.h" -// Comments with Rcout << something here << std::endl; - using namespace Rcpp; [[noreturn]] void fits_throw_exception(const char *func_name, int status) @@ -30,7 +28,22 @@ class fits_file { fits_file(fitsfile *fptr) : m_fptr(fptr) {} fits_file(const fits_file &other) = delete; fits_file &operator=(const fits_file &other) = delete; - fits_file(fits_file &&other) = default; + + fits_file(fits_file &&other) noexcept : m_fptr(other.m_fptr) { + other.m_fptr = nullptr; + } + fits_file &operator=(fits_file &&other) noexcept { + if (this != &other) { + if (m_fptr) { + int status = 0; + fits_close_file(m_fptr, &status); + } + m_fptr = other.m_fptr; + other.m_fptr = nullptr; + } + return *this; + } + ~fits_file() { if (m_fptr) { @@ -49,10 +62,15 @@ class fits_file { return &m_fptr; } - fits_file &operator=(fitsfile *fptr) + // Close any currently-owned handle before taking ownership of a new one. + fits_file &operator=(fitsfile *new_fptr) { - m_fptr = fptr; - return *this; + if (m_fptr) { + int status = 0; + fits_close_file(m_fptr, &status); + } + m_fptr = new_fptr; + return *this; } fitsfile *m_fptr = nullptr; @@ -386,11 +404,6 @@ void Cfits_write_col(Rcpp::String filename, SEXP data, int nrow, int colref=1, i } } -// int CFITS_API ffgkey(fitsfile *fptr, const char *keyname, char *keyval, char *comm, -// int *status); -// -// int CFITS_API ffgky( fitsfile *fptr, int datatype, const char *keyname, void *value, -// char *comm, int *status); // [[Rcpp::export]] SEXP Cfits_read_key(Rcpp::String filename, Rcpp::String keyname, int typecode, int ext=1){ int hdutype; @@ -533,33 +546,34 @@ void Cfits_write_pix(Rcpp::String filename, SEXP data, int ext=1, int datatype= fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - int *int_src = INTEGER(data); - double *dbl_src = REAL(data); - if(datatype == TBYTE){ + int *src = INTEGER(data); std::vector data_b(nelements); - std::transform(int_src, int_src + nelements, data_b.begin(), + std::transform(src, src + nelements, data_b.begin(), [](int v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_b.data()); }else if(datatype == TINT){ - fits_invoke(write_pix, fptr, datatype, fpixel, nelements, int_src); + fits_invoke(write_pix, fptr, datatype, fpixel, nelements, INTEGER(data)); }else if(datatype == TSHORT){ + int *src = INTEGER(data); std::vector data_s(nelements); - std::transform(int_src, int_src + nelements, data_s.begin(), + std::transform(src, src + nelements, data_s.begin(), [](int v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_s.data()); }else if(datatype == TLONG){ + int *src = INTEGER(data); std::vector data_l(nelements); - std::transform(int_src, int_src + nelements, data_l.begin(), + std::transform(src, src + nelements, data_l.begin(), [](int v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_l.data()); }else if(datatype == TLONGLONG){ - fits_invoke(write_pix, fptr, datatype, fpixel, nelements, dbl_src); + fits_invoke(write_pix, fptr, datatype, fpixel, nelements, REAL(data)); }else if(datatype == TDOUBLE){ - fits_invoke(write_pix, fptr, datatype, fpixel, nelements, dbl_src); + fits_invoke(write_pix, fptr, datatype, fpixel, nelements, REAL(data)); }else if(datatype == TFLOAT){ + double *src = REAL(data); std::vector data_f(nelements); - std::transform(dbl_src, dbl_src + nelements, data_f.begin(), + std::transform(src, src + nelements, data_f.begin(), [](double v) { return static_cast(v); }); fits_invoke(write_pix, fptr, datatype, fpixel, nelements, data_f.data()); } @@ -621,7 +635,6 @@ SEXP Cfits_read_img(Rcpp::String filename, int ext=1, int datatype= -32, do_read_img(filename, ext, TDOUBLE, pixel_matrix, nthreads); return(pixel_matrix); }else if (datatype==BYTE_IMG){ - //std::vector pixels(nelements); std::vector pixels(nelements); do_read_img(filename, ext, TBYTE, pixels, nthreads); Rcpp::IntegerVector pixel_matrix(Rcpp::no_init(nelements)); @@ -649,11 +662,11 @@ SEXP Cfits_read_img(Rcpp::String filename, int ext=1, int datatype= -32, // [[Rcpp::export]] SEXP Cfits_read_header(Rcpp::String filename, int ext=1){ - int nkeys, keypos, ii, hdutype; + int nkeys, ii, hdutype; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READONLY); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); + fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); Rcpp::StringVector out(nkeys); char card[FLEN_CARD]; @@ -667,24 +680,23 @@ SEXP Cfits_read_header(Rcpp::String filename, int ext=1){ // [[Rcpp::export]] SEXP Cfits_read_header_raw(Rcpp::String filename, int ext=1){ - int nkeys, keypos, hdutype; + int nkeys, hdutype; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READONLY); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); + fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); Rcpp::StringVector out(1); // fits_hdr2str allocates its own buffer and overwrites the pointer; // do not pre-allocate here or that allocation is leaked. char *header = nullptr; - fits_invoke(hdr2str, fptr, 1, nullptr, 0, &header, &nkeys); - + // RAII wrapper ensures free() is called even if the StringVector assignment throws. + std::unique_ptr header_guard(header, &free); + out[0] = header; - free(header); - return(out); } @@ -733,39 +745,21 @@ SEXP Cfits_read_img_subset(Rcpp::String filename, int ext=1, int datatype= -32, long fpixel[] = {fpixel0, fpixel1, fpixel2, fpixel3}; long lpixel[] = {lpixel0, lpixel1, lpixel2, lpixel3}; - int naxis1 = (lpixel[0] - fpixel[0] + 1); - int naxis2 = (lpixel[1] - fpixel[1] + 1); - int naxis3 = (lpixel[2] - fpixel[2] + 1); - int naxis4 = (lpixel[3] - fpixel[3] + 1); - - // Rcpp::Rcout << naxis1 << naxis2 << naxis3 << naxis4 <<"\n"; - + long naxis1 = (lpixel[0] - fpixel[0] + 1); + long naxis2 = (lpixel[1] - fpixel[1] + 1); + long naxis3 = (lpixel[2] - fpixel[2] + 1); + long naxis4 = (lpixel[3] - fpixel[3] + 1); + if(sparse > 1){ - if(naxis1 > 1){ - naxis1 = 1 + floor((naxis1 - 1)/sparse); - } - - if(naxis2 > 1){ - naxis2 = 1 + floor((naxis2 - 1)/sparse); - } - - if(naxis3 > 1){ - naxis3 = 1 + floor((naxis3 - 1)/sparse); - } - - if(naxis4 > 1){ - naxis4 = 1 + floor((naxis4 - 1)/sparse); - } + if(naxis1 > 1) naxis1 = 1 + (naxis1 - 1) / sparse; + if(naxis2 > 1) naxis2 = 1 + (naxis2 - 1) / sparse; + if(naxis3 > 1) naxis3 = 1 + (naxis3 - 1) / sparse; + if(naxis4 > 1) naxis4 = 1 + (naxis4 - 1) / sparse; } - long nelements = (long)naxis1 * naxis2 * naxis3 * naxis4; + long nelements = naxis1 * naxis2 * naxis3 * naxis4; long inc[] = {sparse, sparse, sparse, sparse}; - - // Rcpp::Rcout << nelements <<"\n"; - // Rcpp::Rcout << inc <<"\n"; - // Rcpp::Rcout << fpixel <<"\n"; - // Rcpp::Rcout << lpixel <<"\n"; - + if (datatype==FLOAT_IMG){ std::vector pixels(nelements); fits_invoke(read_subset, fptr, TFLOAT, fpixel, lpixel, inc, @@ -817,11 +811,7 @@ void Cfits_write_img_subset(Rcpp::String filename, SEXP data, int ext=1, int dat ){ int hdutype; long nelements; - - // Rcpp::Rcout << filename.get_cstring() <<"\n"; - // Rcpp::Rcout << datatype <<"\n"; - // Rcpp::Rcout << naxis <<"\n"; - + fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); fits_invoke(movabs_hdu, fptr, ext, &hdutype); @@ -830,73 +820,62 @@ void Cfits_write_img_subset(Rcpp::String filename, SEXP data, int ext=1, int dat long fpixel_cube[] = {fpixel0, fpixel1, fpixel2}; long fpixel_array[] = {fpixel0, fpixel1, fpixel2, fpixel3}; long *fpixel = (naxis == 1) ? fpixel_vector : (naxis == 2) ? fpixel_image : (naxis == 3 ? fpixel_cube : fpixel_array); - - // Rcpp::Rcout << fpixel0 <<"\n"; - // Rcpp::Rcout << fpixel1 <<"\n"; - // Rcpp::Rcout << fpixel2 <<"\n"; - // Rcpp::Rcout << fpixel3 <<"\n"; - + long lpixel_vector[] = {lpixel0}; long lpixel_image[] = {lpixel0, lpixel1}; long lpixel_cube[] = {lpixel0, lpixel1, lpixel2}; long lpixel_array[] = {lpixel0, lpixel1, lpixel2, lpixel3}; long *lpixel = (naxis == 1) ? lpixel_vector : (naxis == 2) ? lpixel_image : (naxis == 3 ? lpixel_cube : lpixel_array); - // Rcpp::Rcout << lpixel0 <<"\n"; - // Rcpp::Rcout << lpixel1 <<"\n"; - // Rcpp::Rcout << lpixel2 <<"\n"; - // Rcpp::Rcout << lpixel3 <<"\n"; - - int naxis1 = (lpixel[0] - fpixel[0] + 1); - int naxis2 = (lpixel[1] - fpixel[1] + 1); - int naxis3 = (lpixel[2] - fpixel[2] + 1); - int naxis4 = (lpixel[3] - fpixel[3] + 1); - + long naxis1 = (lpixel[0] - fpixel[0] + 1); + long naxis2 = (lpixel[1] - fpixel[1] + 1); + long naxis3 = (lpixel[2] - fpixel[2] + 1); + long naxis4 = (lpixel[3] - fpixel[3] + 1); + if (naxis == 1) { - nelements = (long)naxis1; + nelements = naxis1; } else if (naxis == 2) { - nelements = (long)naxis1 * naxis2; + nelements = naxis1 * naxis2; } else if (naxis == 3) { - nelements = (long)naxis1 * naxis2 * naxis3; + nelements = naxis1 * naxis2 * naxis3; } else if (naxis == 4) { - nelements = (long)naxis1 * naxis2 * naxis3 * naxis4; + nelements = naxis1 * naxis2 * naxis3 * naxis4; } else { Rcpp::stop("naxis=%d doesn't meet condition: 1 <= naxis <= 4", naxis); } - - // Rcpp::Rcout << nelements <<"\n"; - - int *int_src = INTEGER(data); - double *dbl_src = REAL(data); if(datatype == TBYTE){ + int *src = INTEGER(data); std::vector data_b(nelements); - std::transform(int_src, int_src + nelements, data_b.begin(), + std::transform(src, src + nelements, data_b.begin(), [](int v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_b.data()); }else if(datatype == TINT){ - fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, int_src); + fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, INTEGER(data)); }else if(datatype == TSHORT){ + int *src = INTEGER(data); std::vector data_s(nelements); - std::transform(int_src, int_src + nelements, data_s.begin(), + std::transform(src, src + nelements, data_s.begin(), [](int v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_s.data()); }else if(datatype == TLONG){ + int *src = INTEGER(data); std::vector data_l(nelements); - std::transform(int_src, int_src + nelements, data_l.begin(), + std::transform(src, src + nelements, data_l.begin(), [](int v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_l.data()); }else if(datatype == TLONGLONG){ - fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, dbl_src); + fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, REAL(data)); }else if(datatype == TDOUBLE){ - fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, dbl_src); + fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, REAL(data)); }else if(datatype == TFLOAT){ + double *src = REAL(data); std::vector data_f(nelements); - std::transform(dbl_src, dbl_src + nelements, data_f.begin(), + std::transform(src, src + nelements, data_f.begin(), [](double v) { return static_cast(v); }); fits_invoke(write_subset, fptr, datatype, fpixel, lpixel, data_f.data()); } @@ -972,10 +951,10 @@ SEXP Cfits_decode_chksum(Rcpp::String ascii, int complement=0){ // [[Rcpp::export]] int Cfits_read_nkey(Rcpp::String filename, int ext=1){ - int nkeys, keypos, hdutype; + int nkeys, hdutype; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READONLY); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); + fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); return(nkeys); } From ca1f4f8ad65396eae64cd27399e71d6c10f4df9e Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 16:06:22 +0800 Subject: [PATCH 06/21] Final fresh pass and some last tidy ups. Compiles fine (fewer warnings). --- src/Rfits.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index e74d72a..df31e12 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -720,11 +720,11 @@ void Cfits_delete_key(Rcpp::String filename, Rcpp::String keyname, int ext=1){ // [[Rcpp::export]] void Cfits_delete_header(Rcpp::String filename, int ext=1){ - int hdutype, nkeys, keypos, ii; + int hdutype, nkeys, ii; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READWRITE); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); + fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); for (ii = 2; ii <= nkeys; ii++) { fits_invoke(delete_record, fptr, 2); } @@ -925,8 +925,8 @@ SEXP Cfits_get_chksum(Rcpp::String filename){ fits_invoke(get_chksum, fptr, &datasum, &hdusum); Rcpp::NumericVector out(2); out.attr("class") = "integer64"; - std::memcpy(&(out[0]), &datasum, 8); - std::memcpy(&(out[1]), &hdusum, 8); + std::memcpy(&(out[0]), &datasum, sizeof(unsigned long)); + std::memcpy(&(out[1]), &hdusum, sizeof(unsigned long)); return(out); } From f6fb2c88edd24654145531d43893a442e8b32c4a Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 18:16:07 +0800 Subject: [PATCH 07/21] try now --- src/Rfits.cpp | 53 +++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index df31e12..96178f0 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -5,12 +5,13 @@ #include #include "cfitsio/fitsio.h" +#include using namespace Rcpp; [[noreturn]] void fits_throw_exception(const char *func_name, int status) { - char err_msg[30]; + char err_msg[FLEN_STATUS]; fits_get_errstatus(status, err_msg); std::ostringstream os; os << "Error when invoking fits_" << func_name << ": " << err_msg; @@ -129,9 +130,12 @@ static SEXP ensure_lossless_32bit_int(const std::vector &values) if (v > std::numeric_limits::max() || v < std::numeric_limits::min()) { Rcpp::NumericVector output(n); - std::memcpy(&(output[0]), &(values[0]), n * sizeof(long)); - output.attr("class") = "integer64"; - return output; + std::vector values64(n); + std::transform(values.begin(), values.end(), values64.begin(), + [](long v) { return static_cast(v); }); +if (n > 0) std::memcpy(&(output[0]), &(values64[0]), n * sizeof(int64_t)); +output.attr("class") = "integer64"; +return output; } int_out[i] = static_cast(v); } @@ -145,7 +149,7 @@ void Cfits_create_header(Rcpp::String filename, int create_ext=1, int create_fil int nhdu,hdutype; fits_file fptr; int naxis=0; - long *axes = {0}; + long *axes = nullptr; if(create_file == 1){ fits_invoke(create_file, fptr, filename.get_cstring()); @@ -153,7 +157,7 @@ void Cfits_create_header(Rcpp::String filename, int create_ext=1, int create_fil fits_invoke(create_img, fptr, 16, naxis, axes); }else{ if(create_ext == 1){ - fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); + fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); fits_invoke(get_num_hdus, fptr, &nhdu); fits_invoke(movabs_hdu, fptr, nhdu, &hdutype); fits_invoke(create_hdu, fptr); @@ -330,20 +334,17 @@ SEXP Cfits_read_colname(Rcpp::String filename, int colref=1, int ext=2){ fits_invoke(movabs_hdu, fptr, ext, &hdutype); fits_invoke(get_num_cols, fptr, &ncol); - Rcpp::StringVector out(ncol); - + std::vector names; char colname[81]; - int status = 0; - int ii = 0; - while (status != COL_NOT_FOUND && ii < ncol) { - fits_get_colname(fptr, CASEINSEN, (char *)"*", (char *)colname, &colref, &status); + int ref = colref; + while (status != COL_NOT_FOUND && (int)names.size() < ncol) { + fits_get_colname(fptr, CASEINSEN, (char *)"*", colname, &ref, &status); if (status != COL_NOT_FOUND) { - out[ii] = colname; + names.push_back(std::string(colname)); } - ii++; } - return out; +return Rcpp::wrap(names); } // [[Rcpp::export]] @@ -582,13 +583,13 @@ void Cfits_write_pix(Rcpp::String filename, SEXP data, int ext=1, int datatype= template T* start_of(std::vector &output) { - return output.data(); + return output.empty() ? nullptr : output.data(); } template typename Rcpp::Vector::stored_type* start_of(Rcpp::Vector &output) { - return &(output[0]); + return output.size() == 0 ? nullptr : &(output[0]); } static inline void do_read_img(Rcpp::String filename, int ext, int data_type, long start, long count, void *output) @@ -608,8 +609,6 @@ static inline void do_read_img(Rcpp::String filename, int ext, int data_type, Ou #endif R_xlen_t total_elements = output.size(); - R_xlen_t elements_per_thread = total_elements / nthreads; - R_xlen_t remainder = total_elements % nthreads; #ifdef _OPENMP #pragma omp parallel for schedule(static) num_threads(nthreads) @@ -630,11 +629,15 @@ SEXP Cfits_read_img(Rcpp::String filename, int ext=1, int datatype= -32, { long nelements = naxis1 * naxis2 * naxis3 * naxis4; - if (datatype==FLOAT_IMG || datatype == DOUBLE_IMG){ - Rcpp::NumericVector pixel_matrix(Rcpp::no_init(nelements)); - do_read_img(filename, ext, TDOUBLE, pixel_matrix, nthreads); - return(pixel_matrix); - }else if (datatype==BYTE_IMG){ + if (datatype==FLOAT_IMG){ + Rcpp::NumericVector pixel_matrix(Rcpp::no_init(nelements)); + do_read_img(filename, ext, TFLOAT, pixel_matrix, nthreads); + return(pixel_matrix); +}else if (datatype==DOUBLE_IMG){ + Rcpp::NumericVector pixel_matrix(Rcpp::no_init(nelements)); + do_read_img(filename, ext, TDOUBLE, pixel_matrix, nthreads); + return(pixel_matrix); +}else if (datatype==BYTE_IMG){ std::vector pixels(nelements); do_read_img(filename, ext, TBYTE, pixels, nthreads); Rcpp::IntegerVector pixel_matrix(Rcpp::no_init(nelements)); @@ -945,7 +948,7 @@ SEXP Cfits_decode_chksum(Rcpp::String ascii, int complement=0){ fits_decode_chksum((char *)ascii.get_cstring(), complement, &sum); Rcpp::NumericVector out(1); out.attr("class") = "integer64"; - std::memcpy(&(out[0]), &sum, 8); + if (out.size() > 0) std::memcpy(&(out[0]), &sum, sizeof(unsigned long)); return(out); } From b448202da69976d3aecf04f6c9f1af022989efa9 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 22:29:01 +0800 Subject: [PATCH 08/21] Type fix --- src/Rfits.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 96178f0..f1246be 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -631,7 +631,7 @@ SEXP Cfits_read_img(Rcpp::String filename, int ext=1, int datatype= -32, if (datatype==FLOAT_IMG){ Rcpp::NumericVector pixel_matrix(Rcpp::no_init(nelements)); - do_read_img(filename, ext, TFLOAT, pixel_matrix, nthreads); + do_read_img(filename, ext, TDOUBLE, pixel_matrix, nthreads); return(pixel_matrix); }else if (datatype==DOUBLE_IMG){ Rcpp::NumericVector pixel_matrix(Rcpp::no_init(nelements)); From 4067ea9d2eefd2cd6b64cb023a026a92184ece2f Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 22:43:38 +0800 Subject: [PATCH 09/21] Safe --- src/Rfits.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index f1246be..edd4321 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -609,20 +609,24 @@ static inline void do_read_img(Rcpp::String filename, int ext, int data_type, Ou #endif R_xlen_t total_elements = output.size(); + if (total_elements == 0) return; #ifdef _OPENMP + R_xlen_t elems_per_thread = total_elements / nthreads; + R_xlen_t thread_remainder = total_elements % nthreads; + #pragma omp parallel for schedule(static) num_threads(nthreads) for (R_xlen_t i = 0; i < nthreads; i++) { - auto extra = (i < remainder) ? 1 : 0; - auto start = elements_per_thread * i + std::min(remainder, i); - auto count = elements_per_thread + extra; + R_xlen_t extra = (i < thread_remainder) ? 1 : 0; + R_xlen_t start = elems_per_thread * i + std::min(thread_remainder, i); + R_xlen_t count = elems_per_thread + extra; + if (count == 0) continue; do_read_img(filename, ext, data_type, start + 1, count, start_of(output) + start); } #else do_read_img(filename, ext, data_type, 1, total_elements, start_of(output)); #endif } - // [[Rcpp::export]] SEXP Cfits_read_img(Rcpp::String filename, int ext=1, int datatype= -32, long naxis1=100, long naxis2=100, long naxis3=1, long naxis4=1, int nthreads=1) From ba8867f876de7ca6d09055cc0785929d183b71ff Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 23:02:26 +0800 Subject: [PATCH 10/21] Now? --- .github/workflows/main.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6f7aefa..8ddd201 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -44,9 +44,10 @@ jobs: install.packages('hdf5r') install.packages('knitr') install.packages('rmarkdown') + install.packages('rlang') remotes::install_deps(dependencies = TRUE) - remotes::install_cran("rcmdcheck") remotes::install_github("asgr/Rwcs", ref="master") + remotes::install_cran("rcmdcheck") shell: Rscript {0} - uses: r-lib/actions/check-r-package@v2 From ea96187268c771c58df35fd0e87f3d54ea7985db Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 18 Mar 2026 23:51:48 +0800 Subject: [PATCH 11/21] Ignore devel version --- .github/workflows/main.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8ddd201..82c49db 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -19,7 +19,6 @@ jobs: config: - {os: macOS-latest, r: 'release'} - {os: windows-latest, r: 'release'} - - {os: ubuntu-latest, r: 'devel', http-user-agent: 'release'} - {os: ubuntu-latest, r: 'release'} - {os: ubuntu-latest, r: 'oldrel-1'} From 82b35057a60822d0fa17453c28ae76edf30bc313 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Thu, 19 Mar 2026 00:33:18 +0800 Subject: [PATCH 12/21] Comment to stop future edits of this bit --- src/Rfits.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index edd4321..83b710f 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -635,6 +635,9 @@ SEXP Cfits_read_img(Rcpp::String filename, int ext=1, int datatype= -32, if (datatype==FLOAT_IMG){ Rcpp::NumericVector pixel_matrix(Rcpp::no_init(nelements)); + // Read FLOAT_IMG into a double-backed R NumericVector using CFITSIO's TDOUBLE. + // CFITSIO will convert float->double during the read; this avoids allocating + // a temporary float buffer in C++ and keeps the R API returning doubles. do_read_img(filename, ext, TDOUBLE, pixel_matrix, nthreads); return(pixel_matrix); }else if (datatype==DOUBLE_IMG){ From 75b52d9d335613f0b8a392ce7cdbf68010100578 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Thu, 19 Mar 2026 00:59:11 +0800 Subject: [PATCH 13/21] More fixes --- src/Rfits.cpp | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 83b710f..39e2850 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -6,6 +6,9 @@ #include "cfitsio/fitsio.h" #include +#include +#include +#include using namespace Rcpp; @@ -109,11 +112,12 @@ fitsfile *fits_safe_open_file(const char *filename, int mode) std::vector to_string_vector(const Rcpp::CharacterVector &strings) { std::vector c_strings(strings.size()); - std::transform(strings.begin(), strings.end(), c_strings.begin(), - [](const Rcpp::String &string) { - return const_cast(string.get_cstring()); - } - ); + for (R_xlen_t i = 0; i < strings.size(); ++i) { + const char *src = strings[i]; + // strdup allocates with malloc; callers should free() + char *dup = strdup(src ? src : ""); + c_strings[i] = dup; + } return c_strings; } @@ -283,7 +287,9 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, std::vector col(nrow); fits_invoke(read_col, fptr, TLONGLONG, colref, startrow, 1, nrow, &nullval, col.data(), &anynull); Rcpp::NumericVector out(nrow); - std::memcpy(&(out[0]), &(col[0]), nrow * sizeof(int64_t)); + if (nrow > 0) { + std::memcpy(&(out[0]), &(col[0]), nrow * sizeof(int64_t)); + } out.attr("class") = "integer64"; return out; } @@ -379,6 +385,11 @@ void Cfits_create_bintable(Rcpp::String filename, int tfields, fits_invoke(create_tbl, fptr, table_type, 0, tfields, c_ttypes.data(), c_tforms.data(), c_tunits.data(), (char *)extname.get_cstring()); + + // free the duplicated C strings allocated by to_string_vector + for (auto p : c_ttypes) free(p); + for (auto p : c_tforms) free(p); + for (auto p : c_tunits) free(p); } // [[Rcpp::export]] @@ -806,7 +817,7 @@ SEXP Cfits_read_img_subset(Rcpp::String filename, int ext=1, int datatype= -32, fits_invoke(read_subset, fptr, TLONGLONG, fpixel, lpixel, inc, &nullvals, pixels.data(), &anynull); Rcpp::NumericVector pixel_matrix(nelements); - std::memcpy(&(pixel_matrix[0]), &(pixels[0]), nelements * sizeof(int64_t)); + if (nelements > 0) std::memcpy(&(pixel_matrix[0]), &(pixels[0]), nelements * sizeof(int64_t)); pixel_matrix.attr("class") = "integer64"; return(pixel_matrix); } @@ -930,13 +941,15 @@ SEXP Cfits_verify_chksum(Rcpp::String filename, int verbose){ // [[Rcpp::export]] SEXP Cfits_get_chksum(Rcpp::String filename){ - unsigned long datasum, hdusum; + unsigned long datasum_ul, hdusum_ul; fits_file fptr = fits_safe_open_file(filename.get_cstring(), READONLY); - fits_invoke(get_chksum, fptr, &datasum, &hdusum); + fits_invoke(get_chksum, fptr, &datasum_ul, &hdusum_ul); + uint64_t datasum = static_cast(datasum_ul); + uint64_t hdusum = static_cast(hdusum_ul); Rcpp::NumericVector out(2); out.attr("class") = "integer64"; - std::memcpy(&(out[0]), &datasum, sizeof(unsigned long)); - std::memcpy(&(out[1]), &hdusum, sizeof(unsigned long)); + if (out.size() >= 1) std::memcpy(&(out[0]), &datasum, sizeof(uint64_t)); + if (out.size() >= 2) std::memcpy(&(out[1]), &hdusum, sizeof(uint64_t)); return(out); } @@ -951,11 +964,12 @@ SEXP Cfits_encode_chksum(unsigned long sum, int complement=0){ // [[Rcpp::export]] SEXP Cfits_decode_chksum(Rcpp::String ascii, int complement=0){ - unsigned long sum; - fits_decode_chksum((char *)ascii.get_cstring(), complement, &sum); + unsigned long sum_ul; + fits_decode_chksum((char *)ascii.get_cstring(), complement, &sum_ul); + uint64_t sum = static_cast(sum_ul); Rcpp::NumericVector out(1); out.attr("class") = "integer64"; - if (out.size() > 0) std::memcpy(&(out[0]), &sum, sizeof(unsigned long)); + if (out.size() > 0) std::memcpy(&(out[0]), &sum, sizeof(uint64_t)); return(out); } From e3add52c115f4731f13a6336ccff7ac72caa3adb Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Thu, 19 Mar 2026 11:26:02 +0800 Subject: [PATCH 14/21] Final tidy up. Now there are no more recommended fixes (or possible corner case bugs) highlighted. --- src/Rfits.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 39e2850..9989805 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -359,9 +359,14 @@ void Cfits_create_bintable(Rcpp::String filename, int tfields, Rcpp::CharacterVector tunits, Rcpp::String extname, int ext=2, int create_ext=1, int create_file=1, int table_type=2) { - auto c_ttypes = to_string_vector(ttypes); - auto c_tforms = to_string_vector(tforms); - auto c_tunits = to_string_vector(tunits); + struct CStringVecGuard { + std::vector& vec; + CStringVecGuard(std::vector& v) : vec(v) {} + ~CStringVecGuard() { for (auto p : vec) free(p); } + }; + auto c_ttypes = to_string_vector(ttypes); CStringVecGuard g1(c_ttypes); + auto c_tforms = to_string_vector(tforms); CStringVecGuard g2(c_tforms); + auto c_tunits = to_string_vector(tunits); CStringVecGuard g3(c_tunits); int nhdu, hdutype; @@ -385,11 +390,6 @@ void Cfits_create_bintable(Rcpp::String filename, int tfields, fits_invoke(create_tbl, fptr, table_type, 0, tfields, c_ttypes.data(), c_tforms.data(), c_tunits.data(), (char *)extname.get_cstring()); - - // free the duplicated C strings allocated by to_string_vector - for (auto p : c_ttypes) free(p); - for (auto p : c_tforms) free(p); - for (auto p : c_tunits) free(p); } // [[Rcpp::export]] @@ -866,7 +866,7 @@ void Cfits_write_img_subset(Rcpp::String filename, SEXP data, int ext=1, int dat nelements = naxis1 * naxis2 * naxis3 * naxis4; } else { - Rcpp::stop("naxis=%d doesn't meet condition: 1 <= naxis <= 4", naxis); + Rcpp::stop("naxis=" + std::to_string(naxis) + " doesn't meet condition: 1 <= naxis <= 4"); } if(datatype == TBYTE){ From b497e7da30208afacf2957c6f1e36f6d6fbff75f Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Thu, 19 Mar 2026 16:32:44 +0800 Subject: [PATCH 15/21] Just run workflow on main and master --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 82c49db..f146be4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -2,6 +2,7 @@ # Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help on: push: + branches: [main, master] pull_request: branches: [main, master] From bc5142d9f144b412b480f3f55efaf2809bc8ea92 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Tue, 31 Mar 2026 10:35:55 +0800 Subject: [PATCH 16/21] Minor edits (esp since I never actually used Cfits_read_nrow) --- R/Rfits_methods_file.R | 1 - src/RcppExports.cpp | 6 +++--- src/Rfits.cpp | 8 ++++---- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/R/Rfits_methods_file.R b/R/Rfits_methods_file.R index 70c7740..85ae681 100644 --- a/R/Rfits_methods_file.R +++ b/R/Rfits_methods_file.R @@ -20,7 +20,6 @@ Rfits_pixscale = function(filename, ext=1, useraw=TRUE, unit='asec', loc='cen', return(pixscale(temp_header, useraw=useraw, unit=unit, loc='cen', ...)) } - Rfits_pixarea = function(filename, ext=1, useraw=TRUE, unit='asec2', loc='cen', ...){ temp_header = Rfits_read_header(filename=filename, ext=ext) return(pixarea(temp_header, useraw=useraw, unit=unit, loc='cen', ...)) diff --git a/src/RcppExports.cpp b/src/RcppExports.cpp index 0fcc367..d8d3dc3 100644 --- a/src/RcppExports.cpp +++ b/src/RcppExports.cpp @@ -38,7 +38,7 @@ BEGIN_RCPP END_RCPP } // Cfits_read_nrow -int Cfits_read_nrow(Rcpp::String filename, int ext); +long Cfits_read_nrow(Rcpp::String filename, int ext); RcppExport SEXP _Rfits_Cfits_read_nrow(SEXP filenameSEXP, SEXP extSEXP) { BEGIN_RCPP Rcpp::RObject rcpp_result_gen; @@ -105,13 +105,13 @@ BEGIN_RCPP END_RCPP } // Cfits_write_col -void Cfits_write_col(Rcpp::String filename, SEXP data, int nrow, int colref, int ext, int typecode); +void Cfits_write_col(Rcpp::String filename, SEXP data, long nrow, int colref, int ext, int typecode); RcppExport SEXP _Rfits_Cfits_write_col(SEXP filenameSEXP, SEXP dataSEXP, SEXP nrowSEXP, SEXP colrefSEXP, SEXP extSEXP, SEXP typecodeSEXP) { BEGIN_RCPP Rcpp::RNGScope rcpp_rngScope_gen; Rcpp::traits::input_parameter< Rcpp::String >::type filename(filenameSEXP); Rcpp::traits::input_parameter< SEXP >::type data(dataSEXP); - Rcpp::traits::input_parameter< int >::type nrow(nrowSEXP); + Rcpp::traits::input_parameter< long >::type nrow(nrowSEXP); Rcpp::traits::input_parameter< int >::type colref(colrefSEXP); Rcpp::traits::input_parameter< int >::type ext(extSEXP); Rcpp::traits::input_parameter< int >::type typecode(typecodeSEXP); diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 9989805..b0996ed 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -201,7 +201,7 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, // vector>. RAII ensures cleanup even if fits_invoke throws. std::vector storage((long)nrow * (cwidth + 1), '\0'); std::vector data(nrow); - for (int i = 0; i < nrow; i++) data[i] = storage.data() + i * (cwidth + 1); + for (long i = 0; i < nrow; i++) data[i] = storage.data() + i * (cwidth + 1); fits_invoke(read_col, fptr, TSTRING, colref, startrow, 1, nrow, nullptr, data.data(), &anynull); Rcpp::StringVector out(nrow); @@ -303,7 +303,7 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, } // [[Rcpp::export]] -int Cfits_read_nrow(Rcpp::String filename, int ext=2){ +long Cfits_read_nrow(Rcpp::String filename, int ext=2){ int hdutype; long nrow; @@ -393,7 +393,7 @@ void Cfits_create_bintable(Rcpp::String filename, int tfields, } // [[Rcpp::export]] -void Cfits_write_col(Rcpp::String filename, SEXP data, int nrow, int colref=1, int ext=2, int typecode=1){ +void Cfits_write_col(Rcpp::String filename, SEXP data, long nrow, int colref=1, int ext=2, int typecode=1){ int hdutype; fits_file fptr = fits_safe_open_file(filename.get_cstring(), READWRITE); @@ -401,7 +401,7 @@ void Cfits_write_col(Rcpp::String filename, SEXP data, int nrow, int colref=1, i if ( typecode == TSTRING ) { std::vector s_data(nrow); - for (int i = 0; i < nrow; i++) { + for (long i = 0; i < nrow; i++) { s_data[i] = (char*)CHAR(STRING_ELT(data, i)); } fits_invoke(write_col, fptr, typecode, colref, 1, 1, nrow, s_data.data()); From cda45b4751018cf0d66c0edb70cd1934bf11bd24 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Tue, 31 Mar 2026 15:23:53 +0800 Subject: [PATCH 17/21] Change to warning --- src/Rfits.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index b0996ed..bc97baa 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -190,7 +190,7 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, } if (startrow + nrow - 1 > nrow_total) { - Rcpp::stop("Requested range exceeds number of rows in table"); + Rcpp::warning("Requested range exceeds number of rows in table"); } if ( typecode == TSTRING ) { From 4abb4abf093ca7d6f3df373d7a8b8390e03dd2ed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 02:57:15 +0000 Subject: [PATCH 18/21] Fix fits_close_file status ignored in move-assignment and fitsfile* assignment operators Agent-Logs-Url: https://github.com/asgr/Rfits/sessions/cd685580-08b6-4b59-8870-dfb15131a891 Co-authored-by: asgr <5617132+asgr@users.noreply.github.com> --- src/Rfits.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index bc97baa..6a7c5a7 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -36,11 +36,14 @@ class fits_file { fits_file(fits_file &&other) noexcept : m_fptr(other.m_fptr) { other.m_fptr = nullptr; } - fits_file &operator=(fits_file &&other) noexcept { + fits_file &operator=(fits_file &&other) { if (this != &other) { if (m_fptr) { int status = 0; fits_close_file(m_fptr, &status); + if (status) { + fits_throw_exception("close_file", status); + } } m_fptr = other.m_fptr; other.m_fptr = nullptr; @@ -72,6 +75,9 @@ class fits_file { if (m_fptr) { int status = 0; fits_close_file(m_fptr, &status); + if (status) { + fits_throw_exception("close_file", status); + } } m_fptr = new_fptr; return *this; From d3e7438aa7d0b71dcb92abac278540a4510fd071 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 05:51:33 +0000 Subject: [PATCH 19/21] Address remaining PR review comments: hdrpos nullptr, nrow clamp, colname errors, indentation, CI fixes Agent-Logs-Url: https://github.com/asgr/Rfits/sessions/90408e21-b322-4cb6-aad4-c67c5025252c Co-authored-by: asgr <5617132+asgr@users.noreply.github.com> --- .github/workflows/main.yml | 2 +- src/Rfits.cpp | 30 +++++++++++++++++------------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f146be4..a461384 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -2,7 +2,6 @@ # Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help on: push: - branches: [main, master] pull_request: branches: [main, master] @@ -22,6 +21,7 @@ jobs: - {os: windows-latest, r: 'release'} - {os: ubuntu-latest, r: 'release'} - {os: ubuntu-latest, r: 'oldrel-1'} + - {os: ubuntu-latest, r: 'devel'} env: GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 6a7c5a7..9149544 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -143,9 +143,11 @@ static SEXP ensure_lossless_32bit_int(const std::vector &values) std::vector values64(n); std::transform(values.begin(), values.end(), values64.begin(), [](long v) { return static_cast(v); }); -if (n > 0) std::memcpy(&(output[0]), &(values64[0]), n * sizeof(int64_t)); -output.attr("class") = "integer64"; -return output; + if (n > 0) { + std::memcpy(&(output[0]), &(values64[0]), n * sizeof(int64_t)); + } + output.attr("class") = "integer64"; + return output; } int_out[i] = static_cast(v); } @@ -196,7 +198,7 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, } if (startrow + nrow - 1 > nrow_total) { - Rcpp::warning("Requested range exceeds number of rows in table"); + nrow = nrow_total - startrow + 1; } if ( typecode == TSTRING ) { @@ -352,8 +354,10 @@ SEXP Cfits_read_colname(Rcpp::String filename, int colref=1, int ext=2){ int ref = colref; while (status != COL_NOT_FOUND && (int)names.size() < ncol) { fits_get_colname(fptr, CASEINSEN, (char *)"*", colname, &ref, &status); - if (status != COL_NOT_FOUND) { + if (status == 0) { names.push_back(std::string(colname)); + } else if (status != COL_NOT_FOUND) { + fits_throw_exception("get_colname", status); } } return Rcpp::wrap(names); @@ -689,11 +693,11 @@ SEXP Cfits_read_img(Rcpp::String filename, int ext=1, int datatype= -32, // [[Rcpp::export]] SEXP Cfits_read_header(Rcpp::String filename, int ext=1){ - int nkeys, ii, hdutype; + int nkeys, ii, hdutype, keypos; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READONLY); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); + fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); Rcpp::StringVector out(nkeys); char card[FLEN_CARD]; @@ -707,11 +711,11 @@ SEXP Cfits_read_header(Rcpp::String filename, int ext=1){ // [[Rcpp::export]] SEXP Cfits_read_header_raw(Rcpp::String filename, int ext=1){ - int nkeys, hdutype; + int nkeys, hdutype, keypos; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READONLY); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); + fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); Rcpp::StringVector out(1); @@ -747,11 +751,11 @@ void Cfits_delete_key(Rcpp::String filename, Rcpp::String keyname, int ext=1){ // [[Rcpp::export]] void Cfits_delete_header(Rcpp::String filename, int ext=1){ - int hdutype, nkeys, ii; + int hdutype, nkeys, ii, keypos; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READWRITE); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); + fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); for (ii = 2; ii <= nkeys; ii++) { fits_invoke(delete_record, fptr, 2); } @@ -981,10 +985,10 @@ SEXP Cfits_decode_chksum(Rcpp::String ascii, int complement=0){ // [[Rcpp::export]] int Cfits_read_nkey(Rcpp::String filename, int ext=1){ - int nkeys, hdutype; + int nkeys, hdutype, keypos; fits_file fptr; fits_invoke(open_image, fptr, filename.get_cstring(), READONLY); fits_invoke(movabs_hdu, fptr, ext, &hdutype); - fits_invoke(get_hdrpos, fptr, &nkeys, nullptr); + fits_invoke(get_hdrpos, fptr, &nkeys, &keypos); return(nkeys); } From c398e30040c06a264177ee7a6254194853635fc2 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 1 Apr 2026 14:33:28 +0800 Subject: [PATCH 20/21] Some final edits --- src/Rfits.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Rfits.cpp b/src/Rfits.cpp index 9149544..7c70969 100644 --- a/src/Rfits.cpp +++ b/src/Rfits.cpp @@ -198,6 +198,7 @@ SEXP Cfits_read_col(Rcpp::String filename, int colref=1, int ext=2, } if (startrow + nrow - 1 > nrow_total) { + Rcpp::warning("Requested range exceeds number of rows in table"); nrow = nrow_total - startrow + 1; } @@ -354,9 +355,9 @@ SEXP Cfits_read_colname(Rcpp::String filename, int colref=1, int ext=2){ int ref = colref; while (status != COL_NOT_FOUND && (int)names.size() < ncol) { fits_get_colname(fptr, CASEINSEN, (char *)"*", colname, &ref, &status); - if (status == 0) { + if (status != COL_NOT_FOUND) { names.push_back(std::string(colname)); - } else if (status != COL_NOT_FOUND) { + } else { fits_throw_exception("get_colname", status); } } From cdf5b3f0641ed597e9dceed22d02811f688e1300 Mon Sep 17 00:00:00 2001 From: Aaron Robotham Date: Wed, 1 Apr 2026 14:54:44 +0800 Subject: [PATCH 21/21] Get rid of failing Ubuntu (seems to be their issue, not mine) --- .github/workflows/main.yml | 2 +- DESCRIPTION | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a461384..dcf4ef1 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -21,7 +21,7 @@ jobs: - {os: windows-latest, r: 'release'} - {os: ubuntu-latest, r: 'release'} - {os: ubuntu-latest, r: 'oldrel-1'} - - {os: ubuntu-latest, r: 'devel'} + # - {os: ubuntu-latest, r: 'devel'} env: GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} diff --git a/DESCRIPTION b/DESCRIPTION index ad24b22..567a5ab 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,8 +1,8 @@ Package: Rfits Type: Package Title: FITS Readers and Writers -Version: 1.14.5 -Date: 2026-02-26 +Version: 1.15.0 +Date: 2026-04-01 Authors@R: c( person(given='Aaron', family='Robotham', email='aaron.robotham@uwa.edu.au', role=c('aut', 'cre'), comment=c(ORCID='0000-0003-0429-3579')),