[gnuastro-commits] master 54159f28: Arithmetic: name, units and comments of popped operands freed

[Mohammad Akhlaghi]

branch: master
commit 54159f2841ff857204b87d16714567c2f8dc97fc
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Arithmetic: name, units and comments of popped operands freed
    
    Until now, in the Arithmetic program, the possibly existing name, units and
    comments of a dataset were only freed upon reading a new dataset from a
    file. Not when popping an existing dataset from the stack. As a result,
    when running a command like below, Arithmetic would crash:
    
       astarithmetic 50 10 2 makenew set-n n --output new.fits
    
    The reason for this was that the dataset coming from 'makenew' isn't read
    from a file and the 'makenew' operator would set a name, units and comments
    for its output! On the other hand, the 'set-' operator uses the name of a
    dataset to identify which dataset should be used. This conflict caused a
    crash!
    
    With this commit, the steps to remove the name, units and comments from a
    dataset are now done generally (both when reading from a file, and when
    popping from the stack. On the other hand, the extra metadata inserted by
    'makenew' has been removed (Arithmetic is about manipulating data, not
    metdata!).
    
    This bug was reported by Raul Infante-Sainz.
    
    This fixes bug #62683.
---
 NEWS                      |  2 ++
 bin/arithmetic/operands.c | 19 +++++++++----------
 lib/arithmetic-set.c      |  9 +++++----
 lib/arithmetic.c          |  5 ++---
 4 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/NEWS b/NEWS
index 27865415..d24b1d4f 100644
--- a/NEWS
+++ b/NEWS
@@ -176,6 +176,8 @@ See the end of the file for license conditions.
   bug #62680: MakeProfiles using WCS-options when --background is given and
               the background doesn't have WCS. Found with the help of Raul
               Infante-Sainz.
+  bug #62683: Arithmetic's 'set-' operator not working after the 'makenew'
+              operator. Found by Raul Infante-Sainz.
 
 
 
diff --git a/bin/arithmetic/operands.c b/bin/arithmetic/operands.c
index ddea2de1..be563b48 100644
--- a/bin/arithmetic/operands.c
+++ b/bin/arithmetic/operands.c
@@ -149,7 +149,8 @@ operands_add(struct arithmeticparams *p, char *filename, 
gal_data_t *data)
 
       /* If the 'filename' is the name of a dataset, then use a copy of it.
          otherwise, do the basic analysis. */
-      if( filename && gal_arithmetic_set_is_name(p->setprm.named, filename) )
+      if( filename
+          && gal_arithmetic_set_is_name(p->setprm.named, filename) )
         {
           newnode->filename=NULL;
           newnode->data=gal_arithmetic_set_copy_named(&p->setprm, filename);
@@ -243,15 +244,6 @@ operands_pop(struct arithmeticparams *p, char *operator)
                                  p->cp.quietmmap);
       data->ndim=gal_dimension_remove_extra(data->ndim, data->dsize, NULL);
 
-      /* Arithmetic changes the contents of a dataset, so the old name and
-         metadata (in the FITS 'EXTNAME' keyword for example) must not be
-         used beyond this point. Furthermore, in Arithmetic, the 'name'
-         element is used to identify variables (with the 'set-'
-         operator). */
-      if(data->name)    { free(data->name);    data->name=NULL;    }
-      if(data->unit)    { free(data->unit);    data->unit=NULL;    }
-      if(data->comment) { free(data->comment); data->comment=NULL; }
-
       /* When the reference data structure's dimensionality is non-zero, it
          means that this is not the first image read. So, write its basic
          information into the reference data structure for future
@@ -287,6 +279,13 @@ operands_pop(struct arithmeticparams *p, char *operator)
   else
     data=operands->data;
 
+  /* Arithmetic changes the contents of a dataset, so the old name and
+     metadata (in the FITS 'EXTNAME' keyword for example) must not be used
+     beyond this point. Furthermore, in Arithmetic, the 'name' element is
+     used to identify variables (with the 'set-' operator). */
+  if(data->name)    { free(data->name);    data->name=NULL;    }
+  if(data->unit)    { free(data->unit);    data->unit=NULL;    }
+  if(data->comment) { free(data->comment); data->comment=NULL; }
 
   /* Remove this node from the queue, return the data structure. */
   p->operands=operands->next;
diff --git a/lib/arithmetic-set.c b/lib/arithmetic-set.c
index b436c36e..4f9cddbf 100644
--- a/lib/arithmetic-set.c
+++ b/lib/arithmetic-set.c
@@ -102,9 +102,10 @@ gal_arithmetic_set_name(struct gal_arithmetic_set_params 
*p, char *token)
   /* Pop the top operand, then add it to the list of named datasets, but
      only if it is used in later tokens. If it isn't, free the popped
      dataset. The latter case (to define a name, but not use it), is
-     obviously a redundant operation, but that is upto the user, we
-     shouldn't worry about it here. We should just have everything in
-     place, so no crashes occur or no extra memory is consumed. */
+     obviously a redundant operation, but that is upto the user and may
+     happen in scripts where the operands and operators list is
+     automatically generated. We should just have everything in place, so
+     no crashes occur or no extra memory is consumed. */
   if( p->used_later(p, varname) )
     {
       /* Add the top popped operand to the list of names. */
@@ -112,7 +113,7 @@ gal_arithmetic_set_name(struct gal_arithmetic_set_params 
*p, char *token)
 
       /* Write the requested name into this dataset. But note that 'name'
          MUST be already empty. So to be safe, we'll do a sanity check. */
-      if(p->named->name )
+      if(p->named->name)
         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);
diff --git a/lib/arithmetic.c b/lib/arithmetic.c
index e74b05ea..7f7e17ac 100644
--- a/lib/arithmetic.c
+++ b/lib/arithmetic.c
@@ -2639,9 +2639,8 @@ arithmetic_makenew(gal_data_t *sizes)
     }
 
   /* allocate the necessary dataset. */
-  out=gal_data_alloc(NULL, GAL_TYPE_UINT8, ndim, dsize, NULL, 1, minmapsize,
-                     quietmmap, "EMPTY", "NOT-SET",
-                     "Empty dataset created by arithmetic.");
+  out=gal_data_alloc(NULL, GAL_TYPE_UINT8, ndim, dsize, NULL, 1,
+                     minmapsize, quietmmap, NULL, NULL, NULL);
 
   /* Clean up and return. */
   free(dsize);