From e1c0d928f2ec165c33220c67ccdb2903b75d05d7 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@cs.stanford.edu>
Date: Mon, 8 Nov 2021 11:37:02 -0800
Subject: [PATCH] MATRIX SAVE negative tests

---
 src/data/file-handle-def.c     | 35 ++++++++++++++++++++++++++++++++
 src/data/file-handle-def.h     |  2 ++
 src/language/stats/matrix.c    | 14 ++++++-------
 tests/language/stats/matrix.at | 37 ++++++++++++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/src/data/file-handle-def.c b/src/data/file-handle-def.c
index de09ae39a7..09340833be 100644
--- a/src/data/file-handle-def.c
+++ b/src/data/file-handle-def.c
@@ -25,6 +25,7 @@
 
 #include "data/dataset.h"
 #include "data/variable.h"
+#include "libpspp/assertion.h"
 #include "libpspp/cast.h"
 #include "libpspp/compiler.h"
 #include "libpspp/hash-functions.h"
@@ -392,6 +393,40 @@ fh_get_encoding (const struct file_handle *handle)
   return handle->encoding;
 }
 
+/* Returns true if A and B refer to the same file or dataset, false
+   otherwise. */
+bool
+fh_equal (const struct file_handle *a, const struct file_handle *b)
+{
+  if (a->referent != b->referent)
+    return false;
+
+  switch (a->referent)
+    {
+    case FH_REF_FILE:
+      {
+        struct file_identity *a_id = fh_get_identity (a);
+        struct file_identity *b_id = fh_get_identity (b);
+
+        int cmp = fh_compare_file_identities (a_id, b_id);
+
+        fh_free_identity (a_id);
+        fh_free_identity (b_id);
+
+        return cmp == 0;
+      }
+
+    case FH_REF_INLINE:
+      return true;
+
+    case FH_REF_DATASET:
+      return a->ds == b->ds;
+
+    default:
+      NOT_REACHED ();
+    }
+}
+
 /* Returns the dataset handle associated with HANDLE.
    Applicable to only FH_REF_DATASET files. */
 struct dataset *
diff --git a/src/data/file-handle-def.h b/src/data/file-handle-def.h
index 2f944ed1c2..df9a757835 100644
--- a/src/data/file-handle-def.h
+++ b/src/data/file-handle-def.h
@@ -96,6 +96,8 @@ const char *fh_get_name (const struct file_handle *);
 enum fh_referent fh_get_referent (const struct file_handle *);
 const char *fh_get_encoding (const struct file_handle *);
 
+bool fh_equal (const struct file_handle *, const struct file_handle *);
+
 /* Properties of FH_REF_FILE file handles. */
 const char *fh_get_file_name (const struct file_handle *);
 const char *fh_get_file_name_encoding (const struct file_handle *handle);
diff --git a/src/language/stats/matrix.c b/src/language/stats/matrix.c
index d90cd2a326..a145282b12 100644
--- a/src/language/stats/matrix.c
+++ b/src/language/stats/matrix.c
@@ -4370,7 +4370,7 @@ save_file_create (struct matrix_state *s, struct file_handle *fh,
   for (size_t i = 0; i < s->n_save_files; i++)
     {
       struct save_file *sf = s->save_files[i];
-      if (sf->file == fh)
+      if (fh_equal (sf->file, fh))
         {
           fh_unref (fh);
 
@@ -4518,16 +4518,16 @@ save_file_destroy (struct save_file *sf)
         {
           dataset_set_dict (sf->dataset, sf->dict);
           dataset_set_source (sf->dataset, casewriter_make_reader (sf->writer));
-          sf->dict = NULL;
-          sf->writer = NULL;
         }
-
+      else
+        {
+          casewriter_destroy (sf->writer);
+          dict_unref (sf->dict);
+        }
       fh_unref (sf->file);
       string_array_destroy (&sf->variables);
       matrix_expr_destroy (sf->names);
       stringi_set_destroy (&sf->strings);
-      casewriter_destroy (sf->writer);
-      dict_unref (sf->dict);
       free (sf);
     }
 }
@@ -4633,8 +4633,6 @@ matrix_parse_save (struct matrix_state *s)
     }
 
   save->sf = save_file_create (s, fh, &variables, names, &strings);
-  fh = NULL;
-
   return cmd;
 
 error:
diff --git a/tests/language/stats/matrix.at b/tests/language/stats/matrix.at
index c02aa49cbb..d55514c9df 100644
--- a/tests/language/stats/matrix.at
+++ b/tests/language/stats/matrix.at
@@ -3166,4 +3166,41 @@ a,b,c
 1.00,abcd,3.00
 4.00,xyzw,6.00
 ])
+AT_CLEANUP
+
+AT_SETUP([MATRIX - SAVE - negative])
+AT_DATA([matrix.sps], [dnl
+MATRIX.
+SAVE !.
+SAVE 1/OUTFILE=!.
+SAVE 1/VARIABLES=!.
+SAVE 1/NAMES=!.
+SAVE 1/!.
+SAVE 1.
+SAVE 1/OUTFILE='matrix.sav'/NAMES={'a'}/VARIABLES=a.
+SAVE 1/OUTFILE='matrix2.sav'.
+SAVE {1,2}/OUTFILE='matrix2.sav'.
+END MATRIX.
+])
+AT_CHECK([pspp matrix.sps], [1], [dnl
+matrix.sps:2.6: error: SAVE: Syntax error at `!'.
+
+matrix.sps:3.16: error: SAVE: Syntax error at `!': expecting a file name or
+handle name.
+
+matrix.sps:4.18: error: SAVE: Syntax error at `!': expecting variable name.
+
+matrix.sps:5.14: error: SAVE: Syntax error at `!'.
+
+matrix.sps:6.8: error: SAVE: Syntax error at `!': expecting OUTFILE, VARIABLES,
+NAMES, or STRINGS.
+
+matrix.sps:7: error: SAVE: Required subcommand OUTFILE was not specified.
+
+matrix.sps:8: warning: SAVE: VARIABLES and NAMES both specified; ignoring
+NAMES.
+
+error: The first SAVE to `matrix2.sav' within this matrix program had 1
+columns, so a 1×2 matrix cannot be saved to it.
+])
 AT_CLEANUP
\ No newline at end of file
-- 
2.30.2