[gnuastro-commits] master 2ee7ed2 1/2: Freeing of binary arithmetic operators corrected

[Mohammad Akhlaghi]

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

    Freeing of binary arithmetic operators corrected
    
    When the processed output type is different from the type that must be
    returned, the binary operators (that need compilation), would change the
    type (and thus pointers) of the output dataset before cleaning the
    (possibly) temporary left and right operands. This would cause problems
    when the output array is changed because when the operation is done in
    place, the output dataset is used for the left and right operands and we
    would thus try to free the datasets two times!
    
    To solve the problem, changing the final type of the output is done after
    (possibly) cleaning the left and right operands.
---
 lib/arithmetic-binary.c  | 32 +++++++++++++++++++-------------
 lib/arithmetic-onlyint.c | 31 ++++++++++++++++++-------------
 lib/arithmetic.c         |  2 +-
 lib/data.c               | 10 +++++-----
 4 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/lib/arithmetic-binary.c b/lib/arithmetic-binary.c
index db2e2f2..02d7ce1 100644
--- a/lib/arithmetic-binary.c
+++ b/lib/arithmetic-binary.c
@@ -359,6 +359,7 @@ arithmetic_binary(int operator, uint8_t flags, gal_data_t 
*lo,
   l=gal_arithmetic_convert_to_compiled_type(lo, flags);
   r=gal_arithmetic_convert_to_compiled_type(ro, flags);
 
+
   /* Set the output type. For the comparison operators, the output type is
      either 0 or 1, so we will set the output type to `unsigned char' for
      efficient memory and CPU usage. Since the number of operators without
@@ -367,6 +368,7 @@ arithmetic_binary(int operator, uint8_t flags, gal_data_t 
*lo,
      operatrs are given, it will be chosen based on the input types.*/
   otype=gal_arithmetic_binary_out_type(operator, l, r);
 
+
   /* Set the output sizes. */
   minmapsize = ( l->minmapsize < r->minmapsize
                  ? l->minmapsize : r->minmapsize );
@@ -415,19 +417,6 @@ arithmetic_binary(int operator, uint8_t 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. */
-  if( o->type != final_otype )
-    {
-      tmp_o=gal_data_copy_to_new_type(o, final_otype);
-      gal_data_free(o);
-      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
@@ -447,6 +436,23 @@ arithmetic_binary(int operator, uint8_t flags, gal_data_t 
*lo,
       if(r!=ro)           gal_data_free(r);
     }
 
+
+  /* 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.
+
+     IMPORTANT: This has to be done after (possibly) freeing the left and
+     right operands because this step can change the `o' pointer which
+     they may depend on (when working in-place). */
+  if( o->type != final_otype )
+    {
+      tmp_o=gal_data_copy_to_new_type(o, final_otype);
+      gal_data_free(o);
+      o=tmp_o;
+    }
+
   /* Return */
   return o;
 }
diff --git a/lib/arithmetic-onlyint.c b/lib/arithmetic-onlyint.c
index 564c849..9e26382 100644
--- a/lib/arithmetic-onlyint.c
+++ b/lib/arithmetic-onlyint.c
@@ -368,19 +368,6 @@ 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. */
-  if( o->type != final_otype )
-    {
-      tmp_o=gal_data_copy_to_new_type(o, final_otype);
-      gal_data_free(o);
-      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
@@ -400,6 +387,24 @@ arithmetic_onlyint_binary(int operator, unsigned char 
flags,
       if(r!=ro)           gal_data_free(r);
     }
 
+
+  /* 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.
+
+     IMPORTANT: This has to be done after (possibly) freeing the left and
+     right operands because this step can change the `o' pointer which
+     they may depend on (when working in-place). */
+  if( o->type != final_otype )
+    {
+      tmp_o=gal_data_copy_to_new_type(o, final_otype);
+      gal_data_free(o);
+      o=tmp_o;
+    }
+
+
   /* Return */
   return o;
 }
diff --git a/lib/arithmetic.c b/lib/arithmetic.c
index 7fe313b..7f00f09 100644
--- a/lib/arithmetic.c
+++ b/lib/arithmetic.c
@@ -1517,7 +1517,7 @@ gal_arithmetic(int operator, unsigned char flags, ...)
       break;
 
 
-    /* When operator is not recognized. */
+    /* When the operator is not recognized. */
     default:
       error(EXIT_FAILURE, 0, "%s: the argument \"%d\" could not be "
             "interpretted as an operator", __func__, operator);
diff --git a/lib/data.c b/lib/data.c
index 0b1589d..68178b2 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -409,11 +409,11 @@ gal_data_free_contents(gal_data_t *data)
           "`gal_data_free_contents' was a NULL pointer", __func__);
 
   /* Free all the possible allocations. */
-  if(data->name)    { free(data->name);    data->name=NULL;    }
-  if(data->unit)    { free(data->unit);    data->unit=NULL;    }
-  if(data->dsize)   { free(data->dsize);   data->dsize=NULL;   }
-  if(data->wcs)     { wcsfree(data->wcs);  data->wcs=NULL;     }
-  if(data->comment) { free(data->comment); data->comment=NULL; }
+  if(data->name)    { free(data->name);    data->name    = NULL; }
+  if(data->unit)    { free(data->unit);    data->unit    = NULL; }
+  if(data->dsize)   { free(data->dsize);   data->dsize   = NULL; }
+  if(data->wcs)     { wcsfree(data->wcs);  data->wcs     = NULL; }
+  if(data->comment) { free(data->comment); data->comment = NULL; }
 
   /* 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