[gnuastro-commits] master 1ea9ea2 009/125: Data structure mmap information updated

[Mohammad Akhlaghi]

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

    Data structure mmap information updated
    
    As correctly suggested by Antonio Diaz Diaz, the `mmapped' variable in the
    data structure is redundant and will only cause bugs. To check if the array
    is allocated or mapped, it is sufficient to check the `mmapname' pointer:
    if its `NULL', then the array is allocated. So this variable was removed
    from the structure.
    
    On the other hand, in many cases, data structures will be created/derived
    from existing data structures, and it is a burden to pass around the the
    `minmapsize' number. So this variable is now defined in the datastructure
    and derivative data structures will be able to specify if they should be
    allocated or mmap'd.
    
    Some minor corrections were also made in the manual (in the installation
    directory).
---
 bin/arithmetic/arithmetic.c |  6 +++++-
 doc/gnuastro.texi           | 20 +++++++++++---------
 lib/data-arithmetic.h       |  8 +++++++-
 lib/data.c                  | 12 ++++++------
 lib/gnuastro/data.h         | 42 +++++++++++++++++++++++++++---------------
 5 files changed, 56 insertions(+), 32 deletions(-)

diff --git a/bin/arithmetic/arithmetic.c b/bin/arithmetic/arithmetic.c
index d9e5919..a777749 100644
--- a/bin/arithmetic/arithmetic.c
+++ b/bin/arithmetic/arithmetic.c
@@ -156,7 +156,11 @@ reversepolish(struct imgarithparams *p)
                   "interpretted as a FITS file, number, or operator",
                   token->v);
 
-          /* Pop the necessary number of operators. */
+          /* Pop the necessary number of operators. Note that the operators
+             are poped from a linked list (which is last-in-first-out). So
+             for the operators which need a specific order, the first poped
+             operand is actally the last (right most, in in-fix notation)
+             input operand.*/
           if(nop==1)
             d1=pop_operand(p, token->v);
           else if(nop==2)
diff --git a/doc/gnuastro.texi b/doc/gnuastro.texi
index 0487a20..d6b8895 100644
--- a/doc/gnuastro.texi
+++ b/doc/gnuastro.texi
@@ -3441,7 +3441,7 @@ all the variables discussed above. In the examples above, 
the new directory
 was added after the system specified directories. So if the program,
 library or manuals are found in the system wide directories, the user
 directory is no longer searched. If you want to search your local
-installation first, put the new directory before the already existing list
+installation first, put the new directory before the already existing list,
 like the example below.
 
 @example
@@ -3452,14 +3452,16 @@ export 
LD_LIBRARY_PATH=/home/name/.local/lib:$LD_LIBRARY_PATH
 This is good when a library, for example CFITSIO, is already present on the
 system, but the system-wide install wasn't configured with the correct
 configuration flags (see @ref{CFITSIO}), or you want to use a newer version
-and you don't have administrator or root access to update it. With e above
-order @file{LD_LIBRARY_PATH}, the system will first find the CFITSIO you
-installed for yourself and will never reach the system-wide
-installation. However there are important security problems: because all
-important system-wide programs and libraries can be replaced by non-secure
-versions if they also exist in @file{./.local/}. So if you choose this
-order, be sure to keep it clean from executables or libraries with the same
-names as important system programs or libraries.
+and you don't have administrator or root access to update it. If you update
address@hidden with the order shown above, the linker will first
+find the CFITSIO you installed for yourself and use it, it will never reach
+the system-wide installation. However there are important security problems
+with this order: all important system-wide programs and libraries can be
+replaced by non-secure versions with the same file names and put in the
+customized directory (@file{./.local/} in this example). So if you choose
+to search in your customized directory first, be sure to keep it clean from
+executables or libraries with the same names as important system programs
+or libraries.
 
 
 
diff --git a/lib/data-arithmetic.h b/lib/data-arithmetic.h
index da5f766..e7b35cd 100644
--- a/lib/data-arithmetic.h
+++ b/lib/data-arithmetic.h
@@ -291,11 +291,17 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
                                                                         \
                                                                         \
   /* If the output pointer was not set for any reason, allocate it. */  \
+  /* For `mmapsize', note that since its `size_t', it will always be */ \
+  /* Positive. The `-1' that is recommended to give when you want the */\
+  /* value in RAM is actually the largest possible memory location. */  \
+  /* So we just have to choose the smaller minmapsize of the two to */  \
+  /* decide if the output array should be in RAM or not. */             \
   if(o==NULL)                                                           \
     o = gal_data_alloc(NULL, out_type,                                  \
                        l->size>1 ? l->ndim  : r->ndim,                  \
                        l->size>1 ? l->dsize : r->dsize,                 \
-                       0, l->mmapped || r->mmapped);                    \
+                       0, ( l->minmapsize<r->minmapsize                 \
+                            ? l->minmapsize : r->minmapsize) );         \
                                                                         \
                                                                         \
   /* Do the operations based on the different types. */                 \
diff --git a/lib/data.c b/lib/data.c
index e34d9b1..d8b36c7 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -333,7 +333,6 @@ gal_data_mmap(gal_data_t *data)
     error(EXIT_FAILURE, errno, "%s couldn't be closed", filename);
 
   /* Set the mmaped flag to 1 and keep the filename. */
-  data->mmapped=1;
   data->mmapname=filename;
 }
 
@@ -363,6 +362,7 @@ gal_data_alloc(void *array, int type, size_t ndim, long 
*dsize,
   /* Set the basic information we know so far */
   out->ndim=ndim;
   out->type=type;
+  out->minmapsize=minmapsize;
 
 
   /* Initialize the other values */
@@ -410,7 +410,6 @@ gal_data_alloc(void *array, int type, size_t ndim, long 
*dsize,
             out->array = gal_data_malloc_array(out->type, out->size);
 
           /* Set the values. */
-          out->mmapped=0;
           out->mmapname=NULL;
         }
     }
@@ -434,7 +433,7 @@ gal_data_free(gal_data_t *data)
   /* Free all the allocated space and finally the data structure itself. */
   free(data->dsize);
 
-  if(data->mmapped)
+  if(data->mmapname)
     {
       /* Delete the file keeping the array. */
       remove(data->mmapname);
@@ -442,8 +441,9 @@ gal_data_free(gal_data_t *data)
       /* If there is nothing else in the .gnuastro directory, then delete
          the .gnuastro directory too. */
 
-      /* Free the file name space */
+      /* Free the file name space, and set it to NULL. */
       free(data->mmapname);
+      data->mmapname=NULL;
     }
   else
     free(data->array);
@@ -889,7 +889,7 @@ gal_data_copy_to_new_type(gal_data_t *in, int newtype)
   gal_data_t *out;
 
   /* Allocate space for the output type */
-  out=gal_data_alloc(NULL, newtype, in->ndim, in->dsize, 0, in->mmapped);
+  out=gal_data_alloc(NULL, newtype, in->ndim, in->dsize, 0, in->minmapsize);
 
   /* Copy the WCS structure, we need to have a blank WCS structure
      allocated outside of WCSLIB, then copy the contents. */
@@ -1115,7 +1115,7 @@ gal_data_string_to_number(char *string)
 
   /* Return the pointer to the data structure. */
   numarr=gal_data_alloc_number(type, ptr);
-  return gal_data_alloc(numarr, type, 1, dsize, 0, 0);
+  return gal_data_alloc(numarr, type, 1, dsize, 0, -1);
 }
 
 
diff --git a/lib/gnuastro/data.h b/lib/gnuastro/data.h
index 497ef5e..600bbc7 100644
--- a/lib/gnuastro/data.h
+++ b/lib/gnuastro/data.h
@@ -164,24 +164,36 @@ enum gal_data_operators
 
 
 
-/* Main data structure
+/* Main data structure.
 
-   If mmaped==0, it is assumed that the data is allocated (using
-   malloc). The `dsize' array is in the `long' type because CFITSIO uses
-   the long type and this will make it easier to call CFITSIO functions.
- */
+   Notes
+   -----
+
+    - If mmapname==NULL, then the array is allocated (using malloc, in the
+      RAM), otherwise its is mmap'd (is actually a file on the ssd/hdd).
+
+    - minmapsize is stored in the data structure to allow any derivative
+      data structures to follow the same number and decide if they should
+      be mmap'd or allocated.
+
+      - `minmapsize' ==0:  array is definitely mmap'd.
+
+      - `minmapsize' ==-1: array is definitely in RAM.
+
+    - The `dsize' array is in the `long' type because CFITSIO uses the long
+      type and this will make it easier to call CFITSIO functions.*/
 typedef struct
 {
-  void    *array;      /* Array keeping data elements.             */
-  int       type;      /* Type of data (from `gal_data_alltypes'). */
-  size_t    ndim;      /* Number of dimensions in the array.       */
-  long    *dsize;      /* Size of array along each dimension.      */
-  size_t    size;      /* Total number of data-elements.           */
-  int    mmapped;      /* ==1: not in physical RAM, it is mmap'd.  */
-  char *mmapname;      /* File name of the mmap.                   */
-  int   anyblank;      /* ==1: has blank values.                   */
-  int       nwcs;      /* for WCSLIB: no. coord. representations.  */
-  struct wcsprm *wcs;  /* WCS information for this dataset.        */
+  void        *array;  /* Array keeping data elements.               */
+  int           type;  /* Type of data (from `gal_data_alltypes').   */
+  size_t        ndim;  /* Number of dimensions in the array.         */
+  long        *dsize;  /* Size of array along each dimension.        */
+  size_t        size;  /* Total number of data-elements.             */
+  char     *mmapname;  /* File name of the mmap.                     */
+  size_t  minmapsize;  /* Minimum number of bytes to mmap the array. */
+  int       anyblank;  /* ==1: has blank values.                     */
+  int           nwcs;  /* for WCSLIB: no. coord. representations.    */
+  struct wcsprm *wcs;  /* WCS information for this dataset.          */
 } gal_data_t;