[gnuastro-commits] master 7cdea82: Arithmetic's reusing dataset name and collapse memory usage fixed

[Mohammad Akhlaghi]

branch: master
commit 7cdea82fba74f1230dab3bb9464d23cf47e78d04
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    Arithmetic's reusing dataset name and collapse memory usage fixed
    
    When re-using a name for a different dataset, until now we would continue
    parsing the full range of names, even after the old dataset with the
    respective name had been freed! As a result, when there was more than one
    named dataset, this would cause a segmentation fault. So a `break' has been
    added to the `for' loop to stop the parsing after freeing the old dataset.
    
    Also, while investigating this issue, I found another problem with multiple
    uses of the `collapse-XXXX' operators in Arithmetic: if it was called more
    than once (on different datasets, it would just continue decreasing the
    dimensions of the reference dataset to 0 and negative, causing another
    segmentation fault. So before decreasing the reference dataset's
    dimensions, we now check with the dimension of the collapsed dataset.
    
    This fixes bug #55439.
---
 NEWS                        |  1 +
 bin/arithmetic/arithmetic.c | 18 +++++++++++-------
 bin/arithmetic/operands.c   |  6 ++++++
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 4823c2c..3b1ea23 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,7 @@ GNU Astronomy Utilities NEWS                          -*- 
outline -*-
 ** Bugs fixed
   bug #55313: Fits program writing --write values in reverse order
   bug #55333: Fits program crash when --write or --update have no value.
+  bug #55439: Arithmetic segmentation fault when reusing dataset name.
 
 
 
diff --git a/bin/arithmetic/arithmetic.c b/bin/arithmetic/arithmetic.c
index a827db7..595c28e 100644
--- a/bin/arithmetic/arithmetic.c
+++ b/bin/arithmetic/arithmetic.c
@@ -781,13 +781,17 @@ arithmetic_collapse(struct arithmeticparams *p, char 
*token, int operator)
     }
 
 
-  /* We'll also need to correct the size of the reference dataset. We'll
-     use `memcpy' to write the new `dsize' values into the old ones. The
-     dimensions have decreased, so we won't be writing outside of allocated
-     space that `p->refdata.dsize' points to. */
-  p->refdata.ndim -= 1;
-  memcpy( p->refdata.dsize, collapsed->dsize,
-           p->refdata.ndim * (sizeof *p->refdata.dsize) );
+  /* We'll also need to correct the size of the reference dataset if it
+     hasn't been corrected yet. We'll use `memcpy' to write the new `dsize'
+     values into the old ones. The dimensions have decreased, so we won't
+     be writing outside of allocated space that `p->refdata.dsize' points
+     to. */
+  if( p->refdata.ndim != collapsed->ndim )
+    {
+      p->refdata.ndim -= 1;
+      memcpy( p->refdata.dsize, collapsed->dsize,
+              p->refdata.ndim * (sizeof *p->refdata.dsize) );
+    }
 
 
   /* Clean up and add the collapsed dataset to the top of the operands. */
diff --git a/bin/arithmetic/operands.c b/bin/arithmetic/operands.c
index b1062f5..580b8b7 100644
--- a/bin/arithmetic/operands.c
+++ b/bin/arithmetic/operands.c
@@ -150,6 +150,12 @@ operands_set_name(struct arithmeticparams *p, char *token)
       {
         tofree=operands_remove_name(p, varname);
         gal_data_free(tofree);
+
+        /* IMPORTANT: we MUST break here! `tmp' does't point to the right
+           place any more. We can define a `prev' node and modify it on
+           every attempt, but since there is only one dataset with a given
+           name, that is redundant and will just make the program slow. */
+        break;
       }
 
   /* Pop the top operand, then add it to the list of named datasets, but