[gnuastro-commits] master ce73959 030/125: Reading FITS keywords into linked list not array

[Mohammad Akhlaghi]

branch: master
commit ce739598e544a22e0041584d140a6bf39a197053
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    Reading FITS keywords into linked list not array
    
    Until now the `gal_fits_read_keywords_fptr' function would read a keys
    array (assumed to be statically allocated). But since the structure has
    many possible allocations, managing the array would be very complicated. So
    now, `gal_fits_read_keywords_fptr' reads a linked list of data structures
    and fills them based on them in. To make this easy, the old
    `gal_data_add_to_ll' (which took an existing data structure) now takes all
    the parameters to allocate and initialize a data structure. Its old version
    is now called `gal_data_add_existing_to_ll'. Also, a new `gal_data_free_ll'
    function was defined to make it easy to free an existing linked list.
    
    The checking of the final output type in binary operations was done after
    freeing, this would create segmentation faults. So they are now moved
    before the freeing.
---
 bin/arithmetic/arithmetic.c   |   2 +-
 lib/data-arithmetic-binary.c  |  61 +++++++++------
 lib/data-arithmetic-onlyint.c |  27 ++++---
 lib/data.c                    |  44 +++++++++--
 lib/fits.c                    | 178 +++++++++++++++++++++---------------------
 lib/gnuastro/data.h           |  10 ++-
 lib/gnuastro/fits.h           |   6 +-
 lib/txt.c                     |   3 +-
 8 files changed, 190 insertions(+), 141 deletions(-)

diff --git a/bin/arithmetic/arithmetic.c b/bin/arithmetic/arithmetic.c
index d2d5404..3932831 100644
--- a/bin/arithmetic/arithmetic.c
+++ b/bin/arithmetic/arithmetic.c
@@ -322,7 +322,7 @@ reversepolish(struct imgarithparams *p)
               numop=set_number_of_operands(p, pop_operand(p, token->v),
                                            token->v);
               for(i=0;i<numop;++i)
-                gal_data_add_to_ll(&d1, pop_operand(p, token->v));
+                gal_data_add_existing_to_ll(&d1, pop_operand(p, token->v));
               break;
 
             default:
diff --git a/lib/data-arithmetic-binary.c b/lib/data-arithmetic-binary.c
index 1dee7da..1bd24e7 100644
--- a/lib/data-arithmetic-binary.c
+++ b/lib/data-arithmetic-binary.c
@@ -369,6 +369,27 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 /************************************************************************/
 /*************              Top level function          *****************/
 /************************************************************************/
+static int
+set_binary_out_type(int operator, gal_data_t *l, gal_data_t *r)
+{
+  switch(operator)
+    {
+    case GAL_DATA_OPERATOR_PLUS:
+    case GAL_DATA_OPERATOR_MINUS:
+    case GAL_DATA_OPERATOR_MULTIPLY:
+    case GAL_DATA_OPERATOR_DIVIDE:
+      return gal_data_out_type(l, r);
+
+    default:
+      return GAL_DATA_TYPE_UCHAR;
+    }
+  return -1;
+}
+
+
+
+
+
 gal_data_t *
 data_arithmetic_binary(int operator, unsigned char flags, gal_data_t *lo,
                        gal_data_t *ro)
@@ -401,18 +422,7 @@ data_arithmetic_binary(int operator, unsigned char flags, 
gal_data_t *lo,
      a fixed output type (like the conditionals) is less, by `default' we
      will set the output type to `unsigned char', and if any of the other
      operatrs are given, it will be chosen based on the input types.*/
-  switch(operator)
-    {
-    case GAL_DATA_OPERATOR_PLUS:
-    case GAL_DATA_OPERATOR_MINUS:
-    case GAL_DATA_OPERATOR_MULTIPLY:
-    case GAL_DATA_OPERATOR_DIVIDE:
-      otype=gal_data_out_type(l, r);
-      break;
-
-    default:
-      otype=GAL_DATA_TYPE_UCHAR;
-    }
+  otype=set_binary_out_type(operator, l, r);
 
 
   /* Set the output sizes. */
@@ -464,6 +474,20 @@ data_arithmetic_binary(int operator, unsigned char flags, 
gal_data_t *lo,
     }
 
 
+  /* The type of the output dataset (`o->type') was chosen from `l' and `r'
+     (copies of the orignal operands but in a compiled type, not
+     necessarily the original `lo' and `ro' data structures). So we need to
+     to get the final type based on the original operands and check if the
+     final output needs changing. */
+  otype=set_binary_out_type(operator, lo, ro);
+  if( o->type != otype )
+    {
+      tmp_o=gal_data_copy_to_new_type(o, otype);
+      gal_data_free(o, 0);
+      o=tmp_o;
+    }
+
+
   /* Clean up. Note that if the input arrays can be freed, and any of right
      or left arrays needed conversion, `BINARY_CONVERT_TO_COMPILED_TYPE'
      has already freed the input arrays, so only `r' and `l' need
@@ -483,19 +507,6 @@ data_arithmetic_binary(int operator, unsigned char flags, 
gal_data_t *lo,
       if(r!=ro)           gal_data_free(r, 0);
     }
 
-  /* The type of the output dataset (`o->type') was chosen from `l' and `r'
-     (copies of the orignal operands but in a compiled type, not
-     necessarily the original `lo' and `ro' data structures). So we need to
-     to get the final type based on the original operands and check if the
-     final output needs changing. */
-  otype=gal_data_out_type(lo, ro);
-  if( o->type != otype )
-    {
-      tmp_o=gal_data_copy_to_new_type(o, otype);
-      gal_data_free(o, 0);
-      o=tmp_o;
-    }
-
   /* Return */
   return o;
 }
diff --git a/lib/data-arithmetic-onlyint.c b/lib/data-arithmetic-onlyint.c
index 1164fab..7544127 100644
--- a/lib/data-arithmetic-onlyint.c
+++ b/lib/data-arithmetic-onlyint.c
@@ -396,6 +396,20 @@ data_arithmetic_onlyint_binary(int operator, unsigned char 
flags,
     }
 
 
+  /* The type of the output dataset (`o->type') was chosen from `l' and `r'
+     (copies of the orignal operands but in a compiled type, not
+     necessarily the original `lo' and `ro' data structures). So we need to
+     to get the final type based on the original operands and check if the
+     final output needs changing. */
+  otype=gal_data_out_type(lo, ro);
+  if( o->type != otype )
+    {
+      tmp_o=gal_data_copy_to_new_type(o, otype);
+      gal_data_free(o, 0);
+      o=tmp_o;
+    }
+
+
   /* Clean up. Note that if the input arrays can be freed, and any of right
      or left arrays needed conversion, `BINOIN_CONVERT_TO_COMPILED_TYPE'
      has already freed the input arrays, so only `r' and `l' need
@@ -415,19 +429,6 @@ data_arithmetic_onlyint_binary(int operator, unsigned char 
flags,
       if(r!=ro)           gal_data_free(r, 0);
     }
 
-  /* The type of the output dataset (`o->type') was chosen from `l' and `r'
-     (copies of the orignal operands but in a compiled type, not
-     necessarily the original `lo' and `ro' data structures). So we need to
-     to get the final type based on the original operands and check if the
-     final output needs changing. */
-  otype=gal_data_out_type(lo, ro);
-  if( o->type != otype )
-    {
-      tmp_o=gal_data_copy_to_new_type(o, otype);
-      gal_data_free(o, 0);
-      o=tmp_o;
-    }
-
   /* Return */
   return o;
 }
diff --git a/lib/data.c b/lib/data.c
index 6bad8af..f93f45c 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -395,9 +395,10 @@ gal_data_initialize(gal_data_t *data, void *array, int 
type,
   data->next=NULL;
   data->ndim=ndim;
   data->type=type;
+  data->mmapname=NULL;
   data->minmapsize=minmapsize;
-  gal_checkset_allocate_copy(unit, &data->unit);
   gal_checkset_allocate_copy(name, &data->name);
+  gal_checkset_allocate_copy(unit, &data->unit);
   gal_checkset_allocate_copy(comment, &data->comment);
   data->disp_fmt=data->disp_width=data->disp_precision=-1;
 
@@ -445,9 +446,6 @@ gal_data_initialize(gal_data_t *data, void *array, int type,
             data->array = gal_data_calloc_array(data->type, data->size);
           else
             data->array = gal_data_malloc_array(data->type, data->size);
-
-          /* Set the values. */
-          data->mmapname=NULL;
         }
     }
 }
@@ -510,6 +508,7 @@ gal_data_free(gal_data_t *data, int only_contents)
   if(data->wcs)     wcsfree(data->wcs);
   if(data->comment) free(data->comment);
 
+
   /* If the data type is string, then each element in the array is actually
      a pointer to the array of characters, so free them before freeing the
      actual array. */
@@ -564,7 +563,7 @@ gal_data_free(gal_data_t *data, int only_contents)
    structures. Note that if the new node is its self a list, all its nodes
    will be added to the list. */
 void
-gal_data_add_to_ll(gal_data_t **list, gal_data_t *newnode)
+gal_data_add_existing_to_ll(gal_data_t **list, gal_data_t *newnode)
 {
   gal_data_t *tmp=newnode, *toadd;
 
@@ -591,6 +590,25 @@ gal_data_add_to_ll(gal_data_t **list, gal_data_t *newnode)
 
 
 
+void
+gal_data_add_to_ll(gal_data_t **list, void *array, int type, size_t ndim,
+                   long *dsize, struct wcsprm *wcs, int clear,
+                   size_t minmapsize, char *name, char *unit, char *comment)
+{
+  gal_data_t *newnode;
+
+  /* Put all the input information into a new data structure node. */
+  newnode=gal_data_alloc(array, type, ndim, dsize, wcs, clear,
+                         minmapsize, name, unit, comment);
+
+  /* Add the new node to the list. */
+  gal_data_add_existing_to_ll(list, newnode);
+}
+
+
+
+
+
 gal_data_t *
 gal_data_pop_from_ll(gal_data_t **list)
 {
@@ -648,6 +666,22 @@ gal_data_ll_to_array_of_ptrs(gal_data_t *list, size_t *num)
 
 
 
+void
+gal_data_free_ll(gal_data_t *list)
+{
+  struct gal_data_t *tmp;
+  while(list!=NULL)
+    {
+      tmp=list->next;
+      gal_data_free(list, 0);
+      list=tmp;
+    }
+}
+
+
+
+
+
 
 
 
diff --git a/lib/fits.c b/lib/fits.c
index 41e2ce0..96f60e6 100644
--- a/lib/fits.c
+++ b/lib/fits.c
@@ -639,81 +639,84 @@ gal_fits_read_hdu(char *filename, char *hdu, unsigned 
char img0_tab1)
    then the status value will be KEY_NO_EXIST (from CFITSIO).
  */
 void
-gal_fits_read_keywords_fptr(fitsfile *fptr, gal_data_t *keys, size_t num,
+gal_fits_read_keywords_fptr(fitsfile *fptr, gal_data_t *keysll,
                             int readcomment, int readunit)
 {
-  size_t i;
   void *valueptr;
   char **strarray;
+  gal_data_t *tmp;
 
   /* Get the desired keywords. */
-  for(i=0;i<num;++i)
-    {
-      /* Initialize the status: */
-      keys[i].status=0;
-
-      /* Allocate space for the desired type. */
-      keys[i].array=strarray=gal_data_malloc_array(keys[i].type, 1);
-
-      /* When the type is a string, `keys[i].array' will be keeping
-         pointers to a separately allocated piece of memory. So we have to
-         allocate that space here. If its not a string, then the
-         allocated space above is enough to keep the value.*/
-      switch(keys[i].type)
-        {
-        case GAL_DATA_TYPE_STRING:
-          errno=0;
-          valueptr=strarray[0]=malloc(FLEN_VALUE * sizeof *strarray[0]);
-          if(strarray[0]==NULL)
-            error(EXIT_FAILURE, errno, "%zu bytes for strarray[0] in "
-                  "`gal_fits_read_keywords_fprt'",
-                  FLEN_VALUE * sizeof *strarray[0]);
-          break;
-
-        default:
-          valueptr=keys[i].array;
-        }
-
-      /* Allocate space for the keyword comment if necessary.*/
-      if(readcomment)
-        {
-          errno=0;
-          keys[i].comment=malloc(FLEN_COMMENT * sizeof *keys[i].comment);
-          if(keys[i].comment==NULL)
-            error(EXIT_FAILURE, errno, "%zu bytes for keys[i].comment in "
-                  "`gal_fits_read_keywords_fprt'",
-                  FLEN_COMMENT * sizeof *keys[i].comment);
-        }
-      else
-        keys[i].comment=NULL;
-
-      /* Allocate space for the keyword unit if necessary. Note that since
-         there is no precise CFITSIO length for units, we will use the
-         `FLEN_COMMENT' length for units too (theoretically, the unit might
-         take the full remaining area in the keyword). Also note that the
-         unit is only optional, so it needs a separate CFITSIO function
-         call which is done here.*/
-      if(readunit)
-        {
-          errno=0;
-          keys[i].unit=malloc(FLEN_COMMENT * sizeof *keys[i].unit);
-          if(keys[i].unit==NULL)
-            error(EXIT_FAILURE, errno, "%zu bytes for keys[i].unit in "
-                  "`gal_fits_read_keywords_fprt'",
-                  FLEN_COMMENT * sizeof *keys[i].unit);
-          fits_read_key_unit(fptr, keys[i].name, keys[i].unit,
-                             &keys[i].status);
-        }
-      else
-        keys[i].unit=NULL;
-
-      /* Initialize the valueptr to NULL, then Read the keyword and place
-         its value in the poitner. */
-      valueptr=NULL;
-      fits_read_key(fptr, gal_fits_type_to_datatype(keys[i].type),
-                    keys[i].name, valueptr, keys[i].comment,
-                    &keys[i].status);
-    }
+  for(tmp=keysll;tmp!=NULL;tmp=tmp->next)
+    if(tmp->name)
+      {
+        /* Initialize the status: */
+        tmp->status=0;
+
+        /* When the type is a string, `tmp->array' is an array of pointers
+           to a separately allocated piece of memory. So we have to
+           allocate that space here. If its not a string, then the
+           allocated space above is enough to keep the value.*/
+        switch(tmp->type)
+          {
+          case GAL_DATA_TYPE_STRING:
+            errno=0;
+            strarray=tmp->array;
+            valueptr=strarray[0]=malloc(FLEN_VALUE * sizeof *strarray[0]);
+            if(strarray[0]==NULL)
+              error(EXIT_FAILURE, errno, "%zu bytes for strarray[0] in "
+                    "`gal_fits_read_keywords_fprt'",
+                    FLEN_VALUE * sizeof *strarray[0]);
+            break;
+
+          default:
+            valueptr=tmp->array;
+          }
+
+        /* Allocate space for the keyword comment if necessary.*/
+        if(readcomment)
+          {
+            errno=0;
+            tmp->comment=malloc(FLEN_COMMENT * sizeof *tmp->comment);
+            if(tmp->comment==NULL)
+              error(EXIT_FAILURE, errno, "%zu bytes for tmp->comment in "
+                    "`gal_fits_read_keywords_fprt'",
+                    FLEN_COMMENT * sizeof *tmp->comment);
+          }
+        else
+          tmp->comment=NULL;
+
+        /* Allocate space for the keyword unit if necessary. Note that
+           since there is no precise CFITSIO length for units, we will use
+           the `FLEN_COMMENT' length for units too (theoretically, the unit
+           might take the full remaining area in the keyword). Also note
+           that the unit is only optional, so it needs a separate CFITSIO
+           function call which is done here.*/
+        if(readunit)
+          {
+            /* Allocate space for the unit and read it in. */
+            errno=0;
+            tmp->unit=malloc(FLEN_COMMENT * sizeof *tmp->unit);
+            if(tmp->unit==NULL)
+              error(EXIT_FAILURE, errno, "%zu bytes for tmp->unit in "
+                    "`gal_fits_read_keywords_fprt'",
+                    FLEN_COMMENT * sizeof *tmp->unit);
+            fits_read_key_unit(fptr, tmp->name, tmp->unit, &tmp->status);
+
+            /* If the string is empty, free the space and set it to NULL. */
+            if(tmp->unit[0]=='\0') {free(tmp->unit); tmp->unit=NULL;}
+          }
+        else
+          tmp->unit=NULL;
+
+        /* Read the keyword and place its value in the poitner. */
+        fits_read_key(fptr, gal_fits_type_to_datatype(tmp->type),
+                      tmp->name, valueptr, tmp->comment, &tmp->status);
+
+        /* If the comment was empty, free the space and set it to zero. */
+        if(tmp->comment && tmp->comment[0]=='\0')
+          {free(tmp->comment); tmp->comment=NULL;}
+      }
 }
 
 
@@ -723,8 +726,8 @@ gal_fits_read_keywords_fptr(fitsfile *fptr, gal_data_t 
*keys, size_t num,
 /* Same as `gal_fits_read_keywords_fptr', but accepts the filename and HDU
    as input instead of an already opened CFITSIO `fitsfile' pointer. */
 void
-gal_fits_read_keywords(char *filename, char *hdu, gal_data_t *keys,
-                       size_t num, int readcomment, int readunit)
+gal_fits_read_keywords(char *filename, char *hdu, gal_data_t *keysll,
+                       int readcomment, int readunit)
 {
   size_t len;
   int status=0;
@@ -744,7 +747,7 @@ gal_fits_read_keywords(char *filename, char *hdu, 
gal_data_t *keys,
     gal_fits_io_error(status, "reading this FITS file");
 
   /* Read the keywords. */
-  gal_fits_read_keywords_fptr(fptr, keys, num, readcomment, readunit);
+  gal_fits_read_keywords_fptr(fptr, keysll, readcomment, readunit);
 
   /* Close the FITS file. */
   fits_close_file(fptr, &status);
@@ -1239,9 +1242,9 @@ gal_fits_read_img_hdu(char *filename, char *hdu, char 
*maskname,
   size_t i, ndim;
   fitsfile *fptr;
   int status=0, type;
-  long *fpixel, *dsize;
-  char **str, *tmp, *name, *unit;
-  gal_data_t *img, *mask, keys[2];
+  long *fpixel, *dsize, dsize_key=1;
+  char **str, *name=NULL, *unit=NULL;
+  gal_data_t *img, *mask, *keysll=NULL;
 
 
   /* Check HDU for realistic conditions: */
@@ -1274,21 +1277,16 @@ gal_fits_read_img_hdu(char *filename, char *hdu, char 
*maskname,
   /* Read the possibly existing useful keywords. Note that the values are
      in allocated strings in the keys[i] data structures. We don't want to
      allocated them again, so we will just copy the pointers within the
-     `img' data structure and set the pointer in the keys[i] structure to
-     NULL. */
-  keys[0].name = "EXTNAME";
-  keys[1].name = "BUNIT";
-  keys[0].type=keys[1].type=GAL_DATA_TYPE_STRING;
-  gal_fits_read_keywords_fptr(fptr, keys, 2, 0, 0);
-  for(i=0;i<2;++i)
-    {
-      if(keys[0].status==0)
-        { str=keys[i].array; tmp=str[0]; str[0]=NULL; }
-      else tmp=NULL;
-      if(i==0) name=tmp; else unit=tmp;
-      gal_data_free(&keys[i], 1);
-    }
-
+     `img' data structure and set the original value to NULL so it isn't
+     freed. */
+  gal_data_add_to_ll(&keysll, NULL, GAL_DATA_TYPE_STRING, 1, &dsize_key,
+                        NULL, 0, -1, "EXTNAME", NULL, NULL);
+  gal_data_add_to_ll(&keysll, NULL, GAL_DATA_TYPE_STRING, 1, &dsize_key,
+                        NULL, 0, -1, "BUNIT", NULL, NULL);
+  gal_fits_read_keywords_fptr(fptr, keysll, 0, 0);
+  if(keysll->status==0)       {str=keysll->array;       unit=*str; *str=NULL;}
+  if(keysll->next->status==0) {str=keysll->next->array; name=*str; *str=NULL;}
+  gal_data_free_ll(keysll);
 
   /* Allocate the space for the array and for the blank values. */
   img=gal_data_alloc(NULL, type, (long)ndim, dsize, NULL, 0, minmapsize,
diff --git a/lib/gnuastro/data.h b/lib/gnuastro/data.h
index c0155df..170d507 100644
--- a/lib/gnuastro/data.h
+++ b/lib/gnuastro/data.h
@@ -281,7 +281,12 @@ gal_data_free(gal_data_t *data, int only_contents);
 /*************    Data structure as a linked list   ******************/
 /*********************************************************************/
 void
-gal_data_add_to_ll(gal_data_t **list, gal_data_t *newnode);
+gal_data_add_existing_to_ll(gal_data_t **list, gal_data_t *newnode);
+
+void
+gal_data_add_to_ll(gal_data_t **list, void *array, int type, size_t ndim,
+                   long *dsize, struct wcsprm *wcs, int clear,
+                   size_t minmapsize, char *name, char *unit, char *comment);
 
 gal_data_t *
 gal_data_pop_from_ll(struct gal_data_t **list);
@@ -292,7 +297,8 @@ gal_data_num_in_ll(struct gal_data_t *list);
 gal_data_t **
 gal_data_ll_to_array_of_ptrs(gal_data_t *list, size_t *num);
 
-
+void
+gal_data_free_ll(gal_data_t *list);
 
 
 
diff --git a/lib/gnuastro/fits.h b/lib/gnuastro/fits.h
index 21b1251..501acc7 100644
--- a/lib/gnuastro/fits.h
+++ b/lib/gnuastro/fits.h
@@ -163,12 +163,12 @@ gal_fits_read_hdu(char *filename, char *hdu, unsigned 
char img0_tab1);
 /**********            Header keywords             ************/
 /**************************************************************/
 void
-gal_fits_read_keywords_fptr(fitsfile *fptr, gal_data_t *keys, size_t num,
+gal_fits_read_keywords_fptr(fitsfile *fptr, gal_data_t *keysll,
                             int readcomment, int readunit);
 
 void
-gal_fits_read_keywords(char *filename, char *hdu, gal_data_t *keys,
-                       size_t num, int readcomment, int readunit);
+gal_fits_read_keywords(char *filename, char *hdu, gal_data_t *keysll,
+                       int readcomment, int readunit);
 
 void
 gal_fits_add_to_key_ll(struct gal_fits_key_ll **list, int datatype,
diff --git a/lib/txt.c b/lib/txt.c
index f784c9f..0b23069 100644
--- a/lib/txt.c
+++ b/lib/txt.c
@@ -119,8 +119,7 @@ gal_txt_table_info(char *filename, size_t *numcols)
 
       default:
         error(EXIT_FAILURE, 0, "linestatus code %d not recognized in "
-              "`gal_txt_table_info'",
-              linestat);
+              "`gal_txt_table_info'", get_line_stat(line));
       }
 
   /* Clean up, close the file and return. */