[gnuastro-commits] master 03d1a25 15/19: Library (fits.h): corrected segmentation fault in parallel reading

[Mohammad Akhlaghi]

branch: master
commit 03d1a25b915b1ef948f7587e1d589b4a6843b293
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (fits.h): corrected segmentation fault in parallel reading
    
    Until now (in the code that was added in the previous commit), I had
    mistakenly used 'i' in two loops within each other (one for parsing the
    columns given to that thread, another for parsing string columns). So when
    the input had string columns, the library would crash with a segmentation
    fault.
    
    With this commit, a separate 'j' variable was used for the the inner loop
    and this fixed the problem.
---
 lib/fits.c            | 28 +++++++++++++++++-----------
 lib/threads.c         | 17 ++++++++---------
 tests/table/table.txt |  2 +-
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/lib/fits.c b/lib/fits.c
index a41d878..93e2e7d 100644
--- a/lib/fits.c
+++ b/lib/fits.c
@@ -3355,7 +3355,7 @@ fits_tab_read_onecol(void *in_prm)
   gal_data_t *col;
   gal_list_sizet_t *tmp;
   int isfloat, hdutype, anynul=0, status=0;
-  size_t i, c, indout, indin=GAL_BLANK_SIZE_T;
+  size_t i, j, c, indout, indin=GAL_BLANK_SIZE_T;
 
   /* Open the FITS file */
   fptr=gal_fits_hdu_open_format(p->filename, p->hdu, 1);
@@ -3389,16 +3389,16 @@ fits_tab_read_onecol(void *in_prm)
       if(col->type==GAL_TYPE_STRING)
         {
           strarr=col->array;
-          for(i=0;i<p->numrows;++i)
+          for(j=0;j<p->numrows;++j)
             {
               errno=0;
-              strarr[i]=calloc(p->allcols[indin].disp_width+1,
-                               sizeof *strarr[i]);
-              if(strarr[i]==NULL)
+              strarr[j]=calloc(p->allcols[indin].disp_width+1,
+                               sizeof *strarr[j]);
+              if(strarr[j]==NULL)
                 error(EXIT_FAILURE, errno, "%s: allocating %zu bytes for "
                       "strarr[%zu]", __func__,
-                      (p->allcols[indin].disp_width+1) * sizeof *strarr[i],
-                      i);
+                      (p->allcols[indin].disp_width+1) * sizeof *strarr[j],
+                      j);
             }
         }
 
@@ -3433,10 +3433,14 @@ fits_tab_read_onecol(void *in_prm)
                                             p->minmapsize, p->quietmmap);
           status=0;
         }
+      gal_fits_io_error(status, NULL); /* After the 'status' correction. */
 
-      /* Clean up and sanity check. */
+      /* Clean up and sanity check (just note that the blank value for
+         strings, is an array of strings, so we need to free the contents
+         before freeing itself). */
+      if(col->type==GAL_TYPE_STRING)
+        {strarr=blank; free(strarr[0]);}
       if(blank) free(blank);
-      gal_fits_io_error(status, NULL);
 
       /* Everything is fine, put this column in the output array. */
       p->colarray[indout]=col;
@@ -3846,7 +3850,6 @@ gal_fits_tab_write(gal_data_t *cols, gal_list_str_t 
*comments,
   char **ttype, **tform, **tunit;
   int tbltype, numcols=0, status=0;
 
-
   /* Make sure all the input columns have the same number of elements */
   for(col=cols; col!=NULL; col=col->next)
     {
@@ -3915,7 +3918,10 @@ gal_fits_tab_write(gal_data_t *cols, gal_list_str_t 
*comments,
                          i+1, 1, 1, col->size, col->array, blank, &status);
       gal_fits_io_error(status, NULL);
 
-      /* Clean up and Increment the column counter. */
+      /* Clean up and Increment the column counter. Note that unlike
+         reading a column from a table (in 'fits_tab_read_onecol'), the
+         'blank' value for strings here is directly the string, not a
+         pointer to a string. */
       if(blank) free(blank);
       ++i;
     }
diff --git a/lib/threads.c b/lib/threads.c
index d5eca6a..0086dae 100644
--- a/lib/threads.c
+++ b/lib/threads.c
@@ -179,12 +179,12 @@ gal_threads_number()
 
 
 
-/* We have 'numactions' jobs and we want their indexs to be divided
-   between 'numthreads' CPU threads. This function will give each index to
-   a thread such that the maximum difference between the number of
-   images for each thread is 1. The results will be saved in a 2D
-   array of 'outthrdcols' columns and each row will finish with a
-   (size_t) -1, which is larger than any possible index!. */
+/* We have 'numactions' jobs and we want their indexs to be divided between
+   'numthreads' CPU threads. This function will give each index to a thread
+   such that the maximum difference between the number of actions for each
+   thread is 1. The results will be saved in a 2D array of 'outthrdcols'
+   columns and each row will finish with a (size_t)(-1), which is the blank
+   value for size_t (larger than any possible index!). */
 char *
 gal_threads_dist_in_threads(size_t numactions, size_t numthreads,
                             size_t minmapsize, int quietmmap,
@@ -218,8 +218,7 @@ gal_threads_dist_in_threads(size_t numactions, size_t 
numthreads,
         printf("%zu, ", thrds[i*thrdcols+j]);
       printf("\b\b.\n");
     }
-  exit(0);
-  */
+  exit(0); */
 
   /* Return the name of the possibly memory-mapped file. */
   return mmapname;
@@ -353,7 +352,7 @@ gal_threads_spin_off(void *(*worker)(void *), void 
*caller_params,
     error(EXIT_FAILURE, 0, "%s: the number of threads ('numthreads') "
           "cannot be zero", __func__);
 
-  /* Allocate the array of parameters structure structures. */
+  /* Allocate the array of parameters structure. */
   errno=0;
   prm=malloc(numthreads*sizeof *prm);
   if(prm==NULL)
diff --git a/tests/table/table.txt b/tests/table/table.txt
index 305f774..617fbc1 100644
--- a/tests/table/table.txt
+++ b/tests/table/table.txt
@@ -5,7 +5,7 @@
 #  - Blank values in all the columns.
 #  - Empty lines (not comment or data.
 #  - Column information comments are not in the same order as data.
-#  - A column with no information.
+#  - A column (11, i.e., the last) with no information.
 #  - Columns with missing information
 #  - Some blank values different from the internal blank values.