[gnuastro-commits] master 367089d: Library (arithmetic.h): multi-operand operators free next pointer

[Mohammad Akhlaghi]

branch: master
commit 367089de4e417421ab989a01270d7b5a723446a4
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (arithmetic.h): multi-operand operators free next pointer
    
    Until now, the multi-operand operators (that take lists as input, like
    'mean', 'median' and etc: those that take a number for the number of inputs
    and operate on all the pixels) wouldn't set the 'next' pointer of their
    output to NULL. They would just free all the other list elements.
    
    As a result, when a future function/operator wanted to look into the
    datasets, it would think it is a list (because the 'next' element wasn't
    NULL), but the space pointed by 'next' was freed already! This would create
    a segmentation fault!
    
    With this commit, once the multi-operand operation is finished and it is
    busy freeing the input list, when it hits the output dataset, it will set
    its 'next' pointer to NULL. Thus fixing the problem.
    
    In the process, I also discovered that the 'pop_number_of_operands'
    function isn't freeing the popped dataset within the 'SET_NUM_OP' macro!
    That is also fixed here.
    
    This fixes bug #60121.
---
 NEWS                        |  1 +
 bin/arithmetic/arithmetic.c |  6 ++++--
 lib/arithmetic-set.c        |  6 ++++--
 lib/arithmetic.c            | 14 ++++++++------
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index b373a9d..b311d55 100644
--- a/NEWS
+++ b/NEWS
@@ -111,6 +111,7 @@ See the end of the file for license conditions.
 
 ** Bugs fixed
   bug #60082: Arithmetic library crash for integer operators like modulo
+  bug #60121: Arithmetic segfault when multi-operand output given to set-
 
 
 
diff --git a/bin/arithmetic/arithmetic.c b/bin/arithmetic/arithmetic.c
index 3975a7a..963633f 100644
--- a/bin/arithmetic/arithmetic.c
+++ b/bin/arithmetic/arithmetic.c
@@ -63,8 +63,10 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 /***************************************************************/
 /*************          Internal functions         *************/
 /***************************************************************/
-#define SET_NUM_OP(CTYPE) {                                \
-    CTYPE a=*(CTYPE *)(numpop->array); if(a>0) return a;    }
+#define SET_NUM_OP(CTYPE) {            \
+    CTYPE a=*(CTYPE *)(numpop->array); \
+    gal_data_free(numpop);             \
+    if(a>0) return a;    }
 
 static size_t
 pop_number_of_operands(struct arithmeticparams *p, int op, char *token_string,
diff --git a/lib/arithmetic-set.c b/lib/arithmetic-set.c
index 24da3cb..f4d9d68 100644
--- a/lib/arithmetic-set.c
+++ b/lib/arithmetic-set.c
@@ -116,8 +116,10 @@ gal_arithmetic_set_name(struct gal_arithmetic_set_params 
*p, char *token)
         error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at %s to "
               "fix the problem. The 'name' element should be NULL at "
               "this point, but it isn't", __func__, PACKAGE_BUGREPORT);
-      if(p->named->unit)    { free(p->named->unit);    p->named->unit=NULL;    
}
-      if(p->named->comment) { free(p->named->comment); p->named->comment=NULL; 
}
+      if(p->named->unit)
+        { free(p->named->unit); p->named->unit=NULL; }
+      if(p->named->comment)
+        { free(p->named->comment); p->named->comment=NULL; }
       gal_checkset_allocate_copy(varname, &p->named->name);
     }
   else
diff --git a/lib/arithmetic.c b/lib/arithmetic.c
index aed186c..5f427cc 100644
--- a/lib/arithmetic.c
+++ b/lib/arithmetic.c
@@ -1384,9 +1384,9 @@ arithmetic_multioperand(int operator, int flags, 
gal_data_t *list,
         {
         case GAL_ARITHMETIC_OP_QUANTILE:
           if(p1<0 || p1>1)
-            error(EXIT_FAILURE, 0, "%s: the parameter given to the 'quantile' "
-                  "operator must be between (and including) 0 and 1. The "
-                  "given value is: %g", __func__, p1);
+            error(EXIT_FAILURE, 0, "%s: the parameter given to the "
+                  "'quantile' operator must be between (and including) "
+                  "0 and 1. The given value is: %g", __func__, p1);
           break;
         }
     }
@@ -1465,15 +1465,17 @@ arithmetic_multioperand(int operator, int flags, 
gal_data_t *list,
 
 
   /* Clean up and return. Note that the operation might have been done in
-     place. In that case, the top most list element was used. So we need to
-     check before freeing each data structure. */
+     place. In that case, a list element was used. So we need to check
+     before freeing each data structure. If we are on the designated output
+     dataset, we should set its 'next' pointer to NULL so it isn't treated
+     as a list any more by future functions. */
   if(flags & GAL_ARITHMETIC_FREE)
     {
       tmp=list;
       while(tmp!=NULL)
         {
           ttmp=tmp->next;
-          if(tmp!=out) gal_data_free(tmp);
+          if(tmp==out) tmp->next=NULL; else gal_data_free(tmp);
           tmp=ttmp;
         }
       if(params) gal_list_data_free(params);