[gnuastro-commits] master 48a524e0: Library (fits.h): existance is checked before 'fits_open_file'

[Mohammad Akhlaghi]

branch: master
commit 48a524e0e17ae4d36efda70d923cb22d81a35231
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (fits.h): existance is checked before 'fits_open_file'
    
    Until now, we would rely on CFITSIO's 'fits_open_file' function to give a
    non-zero 'status' value when an input file didn't exist. However, in her
    tests, Sepideh Eskandarlou discovered that this CFITSIO function silently
    adds a '.gz' suffix when the actual input file doesn't exist. In Gnuastro's
    workflow, such silent operations are not expected and can cause
    hard-to-find bugs.
    
    With this commit, a check has been added in 'gal_fits_hdu_open' to check
    the existence of the file before calling CFITSIO's 'fits_open_file'. I also
    sent an email to the developers of CFITSIO to suggest adding a non-zero
    status when such silent operations take place.
    
    This bug was reported by Sepideh Eskandarlou.
    
    This fixes bug #64545.
---
 NEWS                 |  2 ++
 bin/fits/fits.c      |  8 ++++++++
 bin/match/ui.c       |  2 +-
 bin/mkcatalog/ui.c   |  9 +++++----
 bin/noisechisel/ui.c |  9 ---------
 bin/segment/ui.c     |  3 ++-
 lib/fits.c           | 34 ++++++++++++++++------------------
 lib/gnuastro/fits.h  |  2 +-
 8 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/NEWS b/NEWS
index f51fa89b..9be4bb3e 100644
--- a/NEWS
+++ b/NEWS
@@ -174,6 +174,8 @@ See the end of the file for license conditions.
   bug #64541: Segment's --keepmaxnearriver has no effect.
   bug #64544: Arithmetic crashes with tofilefree operator value has a
               directory.
+  bug #64545: Accounting for CFITSIO silently appending '.gz' when file
+              doesn't exist. Reported by Sepideh Eskandarlou.
 
 
 
diff --git a/bin/fits/fits.c b/bin/fits/fits.c
index 15017e85..ce3308b8 100644
--- a/bin/fits/fits.c
+++ b/bin/fits/fits.c
@@ -633,6 +633,10 @@ fits_certain_hdu(struct fitsparams *p, int list1has0,
   gal_list_str_t *names=NULL;
   int has=0, naxis, hducounter=1, hdutype, status=0;
 
+  /* Make sure the given file exists: CFITSIO adds '.gz' silently (see more
+     in the comments within 'gal_fits_hdu_open')*/
+  gal_checkset_check_file(p->input->v);
+
   /* Open the FITS file. */
   fits_open_file(&fptr, p->input->v, READONLY, &status);
   gal_fits_io_error(status, NULL);
@@ -721,6 +725,10 @@ fits_list_all_hdus(struct fitsparams *p)
   gal_list_str_t *names=NULL;
   int hducounter=1, hdutype, status=0;
 
+  /* Make sure the given file exists: CFITSIO adds '.gz' silently (see more
+     in the comments within 'gal_fits_hdu_open')*/
+  gal_checkset_check_file(p->input->v);
+
   /* Open the FITS file. */
   fits_open_file(&fptr, p->input->v, READONLY, &status);
   gal_fits_io_error(status, NULL);
diff --git a/bin/match/ui.c b/bin/match/ui.c
index ade4e5e7..1e127a81 100644
--- a/bin/match/ui.c
+++ b/bin/match/ui.c
@@ -831,7 +831,7 @@ ui_read_kdtree(struct matchparams *p)
   /* Read the k-d tree root. */
   keysll[0].type=GAL_TYPE_SIZE_T;
   keysll[0].name=MATCH_KDTREE_ROOT_KEY;
-  gal_fits_key_read(p->kdtree, p->kdtreehdu, keysll, 0, 0);
+  gal_fits_key_read(p->kdtree, p->kdtreehdu, keysll, 0, 0, "--kdtreehdu");
   if(keysll[0].status)
     error(EXIT_FAILURE, 0, "%s (hdu: %s, that was given to "
           "'--kdtree') doesn't have the '%s' keyword, or it "
diff --git a/bin/mkcatalog/ui.c b/bin/mkcatalog/ui.c
index c8e97f80..c9e3136b 100644
--- a/bin/mkcatalog/ui.c
+++ b/bin/mkcatalog/ui.c
@@ -848,7 +848,6 @@ ui_read_labels(struct mkcatalogparams *p)
   p->objects->ndim=gal_dimension_remove_extra(p->objects->ndim,
                                               p->objects->dsize, NULL);
 
-
   /* Make sure it has an integer type. */
   ui_check_type_int(p->objectsfile, p->cp.hdu, p->objects->type);
 
@@ -888,7 +887,7 @@ ui_read_labels(struct mkcatalogparams *p)
   keys[0].name="NUMLABS";
   keys[0].type=GAL_TYPE_SIZE_T;
   keys[0].array=&p->numobjects;
-  gal_fits_key_read(p->objectsfile, p->cp.hdu, keys, 0, 0);
+  gal_fits_key_read(p->objectsfile, p->cp.hdu, keys, 0, 0, "--hdu");
   if(keys[0].status) /* status!=0: the key couldn't be read by CFITSIO. */
     {
       tmp=gal_statistics_maximum(p->objects);
@@ -956,7 +955,8 @@ ui_read_labels(struct mkcatalogparams *p)
       keys[0].name="CLUMPSN";               keys[1].name="NUMLABS";
       keys[0].type=GAL_TYPE_FLOAT32;        keys[1].type=GAL_TYPE_SIZE_T;
       keys[0].array=&p->clumpsn;            keys[1].array=&p->numclumps;
-      gal_fits_key_read(p->usedclumpsfile, p->clumpshdu, keys, 0, 0);
+      gal_fits_key_read(p->usedclumpsfile, p->clumpshdu, keys, 0, 0,
+                        "--clumpshdu");
       if(keys[0].status) p->clumpsn=NAN;
       if(keys[1].status) p->numclumps=ui_num_clumps(p);
 
@@ -1487,7 +1487,8 @@ ui_preparations_read_keywords(struct mkcatalogparams *p)
           keys[0].name="MINSTD";          keys[1].name="MEDSTD";
           keys[0].type=GAL_TYPE_FLOAT32;  keys[1].type=GAL_TYPE_FLOAT32;
           keys[0].array=&minstd;          keys[1].array=&p->medstd;
-          gal_fits_key_read(p->usedstdfile, p->stdhdu, keys, 0, 0);
+          gal_fits_key_read(p->usedstdfile, p->stdhdu, keys, 0, 0,
+                            "--stdhdu");
 
           /* If the two keywords couldn't be read. We don't want to slow
              down the user for the median (which needs sorting). So we'll
diff --git a/bin/noisechisel/ui.c b/bin/noisechisel/ui.c
index d5220a4e..c7e175bf 100644
--- a/bin/noisechisel/ui.c
+++ b/bin/noisechisel/ui.c
@@ -253,9 +253,6 @@ ui_read_check_only_options(struct noisechiselparams *p)
   /* Kernel checks. */
   if(p->kernelname && strcmp(p->kernelname, UI_NO_CONV_KERNEL_NAME))
     {
-      /* Check if it exists. */
-      gal_checkset_check_file(p->kernelname);
-
       /* If its FITS, see if a HDU has been provided. */
       if( gal_fits_file_recognized(p->kernelname) && p->khdu==NULL )
         error(EXIT_FAILURE, 0, "no HDU specified for kernel. When the "
@@ -268,9 +265,6 @@ ui_read_check_only_options(struct noisechiselparams *p)
   /* Wide kernel checks. */
   if(p->widekernelname)
     {
-      /* Check if it exists. */
-      gal_checkset_check_file(p->widekernelname);
-
       /* If its FITS, see if a HDU has been provided. */
       if( gal_fits_file_recognized(p->widekernelname) && p->whdu==NULL )
         error(EXIT_FAILURE, 0, "no HDU specified for the given wide kernel "
@@ -326,9 +320,6 @@ ui_check_options_and_arguments(struct noisechiselparams *p)
   /* Basic input file checks. */
   if(p->inputname)
     {
-      /* Check if it exists. */
-      gal_checkset_check_file(p->inputname);
-
       /* If its FITS, see if a HDU has been provided. */
       if( gal_fits_file_recognized(p->inputname) && p->cp.hdu==NULL )
         error(EXIT_FAILURE, 0, "no HDU specified for input. When the input "
diff --git a/bin/segment/ui.c b/bin/segment/ui.c
index bb254189..64f213ee 100644
--- a/bin/segment/ui.c
+++ b/bin/segment/ui.c
@@ -807,7 +807,8 @@ ui_read_std_and_sky(struct segmentparams *p)
       keys[1].array=&p->minstd;     keys[1].name="MINSTD";
       keys[2].array=&p->maxstd;     keys[2].name="MAXSTD";
       keys[0].type=keys[1].type=keys[2].type=GAL_TYPE_FLOAT32;
-      gal_fits_key_read(p->usedstdname, p->stdhdu, keys, 0, 0);
+      gal_fits_key_read(p->usedstdname, p->stdhdu, keys, 0, 0,
+                        "--stdhdu");
       if(keys[0].status) p->medstd=NAN;
       if(keys[1].status) p->minstd=NAN;
       if(keys[2].status) p->maxstd=NAN;
diff --git a/lib/fits.c b/lib/fits.c
index 8f0a93d7..df377294 100644
--- a/lib/fits.c
+++ b/lib/fits.c
@@ -213,7 +213,8 @@ gal_fits_file_recognized(char *filename)
   /* CFITSIO has some special conventions for file names (for example if
      its '-', which can happen in the Arithmetic program). So before
      passing to CFITSIO, we'll check if a file is actually associated with
-     the string. */
+     the string. For another example see the comments in
+     'gal_fits_hdu_open' about a '.gz' suffix. */
   errno=0;
   tmpfile = fopen(filename, "r");
   if(tmpfile) /* The file existed and opened. */
@@ -696,6 +697,10 @@ gal_fits_hdu_num(char *filename)
   fitsfile *fptr;
   int num, status=0;
 
+  /* Make sure the given file exists: CFITSIO adds '.gz' silently (see more
+     in the comments within 'gal_fits_hdu_open')*/
+  gal_checkset_check_file(filename);
+
   /* We don't need to check for an error everytime, because we don't
      make any non CFITSIO usage of the output. It is necessary to
      check every CFITSIO call, only when you will need to use the
@@ -829,6 +834,13 @@ gal_fits_hdu_open(char *filename, char *hdu, int iomode, 
int exitonerror,
   fitsfile *fptr;
   char *hduon = hdu_option_name ? hdu_option_name : "--hdu";
 
+  /* Make sure the file exists. This is necessary because CFITSIO
+     automatically appends a '.gz' when a file with that extension already
+     exists! For example if the user asked for 'abc.fits', but the
+     directory only includes 'abc.fits.gz', CFITSIO will open that instead
+     (without any status value). */
+  gal_checkset_check_file(filename);
+
   /* Add hdu to filename: */
   if( asprintf(&ffname, "%s[%s#]", filename, hdu)<0 )
     {
@@ -1445,24 +1457,13 @@ gal_fits_key_read_from_ptr(fitsfile *fptr, gal_data_t 
*keysll,
    as input instead of an already opened CFITSIO 'fitsfile' pointer. */
 void
 gal_fits_key_read(char *filename, char *hdu, gal_data_t *keysll,
-                  int readcomment, int readunit)
+                  int readcomment, int readunit, char *hdu_option_name)
 {
-  size_t len;
   int status=0;
-  char *ffname;
   fitsfile *fptr;
 
-  /* Add hdu to filename: */
-  errno=0;
-  len=strlen(filename)+strlen(hdu)+4;
-  ffname=malloc(len*sizeof *ffname);
-  if(ffname==NULL)
-    error(EXIT_FAILURE, errno, "%s: %zu characters", __func__, len);
-  sprintf(ffname, "%s[%s#]", filename, hdu);
-
-  /* Open the FITS file: */
-  if( fits_open_file(&fptr, ffname, READONLY, &status) )
-    gal_fits_io_error(status, "reading this FITS file");
+  /* Open the input HDU. */
+  fptr=gal_fits_hdu_open(filename, hdu, READONLY, 1, hdu_option_name);
 
   /* Read the keywords. */
   gal_fits_key_read_from_ptr(fptr, keysll, readcomment, readunit);
@@ -1470,9 +1471,6 @@ gal_fits_key_read(char *filename, char *hdu, gal_data_t 
*keysll,
   /* Close the FITS file. */
   fits_close_file(fptr, &status);
   gal_fits_io_error(status, NULL);
-
-  /* Clean up. */
-  free(ffname);
 }
 
 
diff --git a/lib/gnuastro/fits.h b/lib/gnuastro/fits.h
index 0a3725ff..41f80e20 100644
--- a/lib/gnuastro/fits.h
+++ b/lib/gnuastro/fits.h
@@ -207,7 +207,7 @@ gal_fits_key_read_from_ptr(fitsfile *fptr, gal_data_t 
*keysll,
 
 void
 gal_fits_key_read(char *filename, char *hdu, gal_data_t *keysll,
-                  int readcomment, int readunit);
+                  int readcomment, int readunit, char *hdu_option_name);
 
 void
 gal_fits_key_list_add(gal_fits_list_key_t **list, uint8_t type,