[gnuastro-commits] master 3f7945d: Library (pointer.h): unused RAM decreased to 250MB

[Mohammad Akhlaghi]

branch: master
commit 3f7945d8ed9a9af8243be059780fb911e57c6651
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (pointer.h): unused RAM decreased to 250MB
    
    Until now, the minimum amount of RAM in the system that Gnuastro programs
    and libraries would leave free was 750MB. However, on a recent build in
    Debian's tracking system, running of MakeNoise failed in 'make check' on
    the Hurd kernel (which is still based on a 32-bit address space and thus
    has a very limited RAM of 830MB). Initially I thought this crash was caused
    due to parallel operations of that Debian Hurd system.
    
    With this commit, the minimum available memory that will never be used by
    Gnuastro was set to 250MB, which is still more than enough for basic system
    operation (given that the system was already occupying its necessary RAM
    before the Gnuastro program was running). This is only for operations in
    parallel to the program.
    
    But after the fix, I noticed a more simplier cause of this problem: instead
    of passing 'EXIT_SUCCESS' to the 'error' function (so it doesn't crash the
    program, but just prints a warning), I had passed 'EXIT_FAILURE' after the
    warning of not finding the 'MemAvailable' keyword.
    
    Also, in the process, I done some leak-checking in MakeNoise to make sure
    there was no memory issues there and found and fixed a few leaking
    cases. Afterwards, I also done a similar leak test on NoiseChisel (without
    any parameters) and fixed a few leaks there too.
---
 bin/mknoise/mknoise.c       |  5 +++--
 bin/noisechisel/threshold.c |  1 +
 bin/noisechisel/ui.c        | 15 +++++++++------
 lib/checkset.c              | 10 +++++-----
 lib/options.c               | 15 +++++++++------
 lib/type.c                  | 20 ++++++++++----------
 6 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/bin/mknoise/mknoise.c b/bin/mknoise/mknoise.c
index 021420b..6731ed7 100644
--- a/bin/mknoise/mknoise.c
+++ b/bin/mknoise/mknoise.c
@@ -109,11 +109,12 @@ convertsaveoutput(struct mknoiseparams *p)
                             "Random number generator (by GSL) seed.",
                             0, NULL, 0);
 
-  /* Save the output: */
+  /* Save the output: first convert it to the desired type,  */
+  if(p->input->name) { free(p->input->name); p->input->name=NULL; }
   p->input=gal_data_copy_to_new_type_free(p->input, p->cp.type);
   p->input->name="NOISED";
   gal_fits_img_write(p->input, p->cp.output, headers, PROGRAM_NAME);
-  p->input->name=NULL;
+  p->input->name=NULL; /* because we didn't allocate it. */
 
   /* Write the configuration keywords. */
   gal_fits_key_write_filename("input", p->inputname, &p->cp.okeys, 1);
diff --git a/bin/noisechisel/threshold.c b/bin/noisechisel/threshold.c
index af77140..fa7ea39 100644
--- a/bin/noisechisel/threshold.c
+++ b/bin/noisechisel/threshold.c
@@ -685,6 +685,7 @@ threshold_quantile_find_apply(struct noisechiselparams *p)
   nval=((size_t *)(num->array))[0];
   if( nval < cp->interpnumngb )
     threshold_good_error(nval, 1, cp->interpnumngb);
+  gal_data_free(num);
 
 
   /* Interpolate and smooth the derived values. */
diff --git a/bin/noisechisel/ui.c b/bin/noisechisel/ui.c
index 9e2bf14..8358230 100644
--- a/bin/noisechisel/ui.c
+++ b/bin/noisechisel/ui.c
@@ -848,12 +848,15 @@ ui_free_report(struct noisechiselparams *p, struct 
timeval *t1)
   free(p->maxtsize);
   free(p->maxltsize);
   free(p->cp.output);
-  if(p->skyname)          free(p->skyname);
-  if(p->detskyname)       free(p->detskyname);
-  if(p->qthreshname)      free(p->qthreshname);
-  if(p->detsn_s_name)     free(p->detsn_s_name);
-  if(p->detsn_d_name)     free(p->detsn_d_name);
-  if(p->detectionname)    free(p->detectionname);
+  if(p->khdu) free(p->khdu);
+  if(p->whdu) free(p->whdu);
+  if(p->chdu) free(p->chdu);
+  if(p->skyname) free(p->skyname);
+  if(p->detskyname) free(p->detskyname);
+  if(p->qthreshname) free(p->qthreshname);
+  if(p->detsn_s_name) free(p->detsn_s_name);
+  if(p->detsn_d_name) free(p->detsn_d_name);
+  if(p->detectionname) free(p->detectionname);
 
   /* Free the allocated datasets. */
   gal_data_free(p->sky);
diff --git a/lib/checkset.c b/lib/checkset.c
index 8e52f42..4247938 100644
--- a/lib/checkset.c
+++ b/lib/checkset.c
@@ -195,9 +195,9 @@ gal_checkset_ram_available(int quietmmap)
 
       /* The file existed but a keyname couldn't be found. In this case we
          should inform the user to be aware that we can't automatically
-         determine the available memory.*/
+         determine the available memory. */
       if(keyfound==0 && quietmmap==0)
-        error(EXIT_FAILURE, 0, "WARNING: %s: didn't contain a '%s' keyword "
+        error(EXIT_SUCCESS, 0, "WARNING: %s: didn't contain a '%s' keyword "
               "hence the amount of available RAM couldn't be determined. "
               "If a large volume of data is provided, the program may "
               "crash. Please contact us at '%s' to fix the problem",
@@ -222,7 +222,7 @@ gal_checkset_need_mmap(size_t bytesize, size_t minmapsize, 
int quietmmap)
   int needmmap=0;
   size_t availableram;
   size_t minimumtommap=10000000;
-  size_t mustremainfree=750000000;
+  size_t mustremainfree=250000000;
 
   /* In case the given minmapsize is smaller than the default value of
      'minimumtomap', then correct 'minimumtomap' to be the same as
@@ -234,8 +234,8 @@ gal_checkset_need_mmap(size_t bytesize, size_t minmapsize, 
int quietmmap)
       /* Let the user know that this is not a good choice and can cause
          other problems. */
       if(!quietmmap)
-        error(EXIT_SUCCESS, 0, "it is recommended that minmapsize have "
-              "a value larger than %zu (it is currently %zu), see "
+        error(EXIT_SUCCESS, 0, "WARNING: it is recommended that minmapsize "
+              "have a value larger than %zu (it is currently %zu), see "
               "\"Memory management\" section in the Gnuastro book for "
               "more. To disable this warning, please use the option "
               "'--quiet-mmap'", minimumtommap, minmapsize);
diff --git a/lib/options.c b/lib/options.c
index 1c4e589..82e8f3f 100644
--- a/lib/options.c
+++ b/lib/options.c
@@ -2788,6 +2788,7 @@ options_as_fits_keywords_write(gal_fits_list_key_t **keys,
 {
   size_t i;
   void *vptr;
+  int vptrfree;
   uint8_t vtype;
   char *name, *doc;
   gal_list_str_t *tmp;
@@ -2803,8 +2804,8 @@ options_as_fits_keywords_write(gal_fits_list_key_t **keys,
               /* 'name' and 'doc' have a 'const' qualifier. */
               gal_checkset_allocate_copy(options[i].name, &name);
               gal_checkset_allocate_copy(options[i].doc,  &doc);
-              gal_fits_key_list_add(keys, GAL_TYPE_STRING, name, 1, tmp->v,
-                                    0, doc, 1, NULL, 0);
+              gal_fits_key_list_add(keys, GAL_TYPE_STRING, name, 1,
+                                    tmp->v, 0, doc, 1, NULL, 0);
             }
         /* Normal types. */
         else
@@ -2812,11 +2813,13 @@ options_as_fits_keywords_write(gal_fits_list_key_t 
**keys,
             /* If the option is associated with a special function for
                reading and writing, we'll need to write the value as a
                string. */
+            vptrfree=0;
             if(options[i].func)
               {
+                vptrfree=1;
                 vtype=GAL_TYPE_STRING;
-                vptr=options[i].func(&options[i], NULL, NULL, (size_t)(-1),
-                                      cp->program_struct);
+                vptr=options[i].func(&options[i], NULL, NULL,
+                                     (size_t)(-1), cp->program_struct);
               }
             else
               {
@@ -2834,8 +2837,8 @@ options_as_fits_keywords_write(gal_fits_list_key_t **keys,
             else
               {
                 gal_checkset_allocate_copy(options[i].doc,  &doc);
-                gal_fits_key_list_add(keys, vtype, name, 1, vptr, 0, doc, 1,
-                                      NULL, 0);
+                gal_fits_key_list_add(keys, vtype, name, 1, vptr,
+                                      vptrfree, doc, 1, NULL, 0);
               }
           }
       }
diff --git a/lib/type.c b/lib/type.c
index 668f10b..f70c58d 100644
--- a/lib/type.c
+++ b/lib/type.c
@@ -502,8 +502,8 @@ gal_type_from_string(void **out, char *string, uint8_t type)
         status=1;
       else
         {
-          if(type==GAL_TYPE_FLOAT32) *(float *) value=d;
-          else                            *(double *) value=d;
+          if(type==GAL_TYPE_FLOAT32) *(float  *) value=d;
+          else                       *(double *) value=d;
         }
       break;
 
@@ -516,10 +516,10 @@ gal_type_from_string(void **out, char *string, uint8_t 
type)
         switch(type)
           {
           /* The signed values can easily be put in. */
-          case GAL_TYPE_INT8:         *(int8_t *)    value = l; break;
-          case GAL_TYPE_INT16:        *(int16_t *)   value = l; break;
-          case GAL_TYPE_INT32:        *(int32_t *)   value = l; break;
-          case GAL_TYPE_INT64:        *(int64_t *)   value = l; break;
+          case GAL_TYPE_INT8:  *(int8_t  *) value = l; break;
+          case GAL_TYPE_INT16: *(int16_t *) value = l; break;
+          case GAL_TYPE_INT32: *(int32_t *) value = l; break;
+          case GAL_TYPE_INT64: *(int64_t *) value = l; break;
 
           /* For the unsigned types, the value has to be positive, so if
              the input was negative, then just return a status of one and
@@ -530,10 +530,10 @@ gal_type_from_string(void **out, char *string, uint8_t 
type)
             else
               switch(type)
                 {
-                case GAL_TYPE_UINT8:  *(uint8_t *)   value=l;   break;
-                case GAL_TYPE_UINT16: *(uint16_t *)  value=l;   break;
-                case GAL_TYPE_UINT32: *(uint32_t *)  value=l;   break;
-                case GAL_TYPE_UINT64: *(uint64_t *)  value=l;   break;
+                case GAL_TYPE_UINT8:  *(uint8_t  *) value=l;   break;
+                case GAL_TYPE_UINT16: *(uint16_t *) value=l;   break;
+                case GAL_TYPE_UINT32: *(uint32_t *) value=l;   break;
+                case GAL_TYPE_UINT64: *(uint64_t *) value=l;   break;
                 default:
                   error(EXIT_FAILURE, 0, "%s: type code %d not recognized",
                         __func__, type);