message: Make msg_dup() copy and msg_destroy() free the file name.

This fixes a PSPPIRE bug in its message handling (found by inspection).  It
makes copies of messages to use later.  The file names were not being
copied, so in the meantime it was posible that they would be freed,
yielding a use-after-free error.

Since doing this sensibly required changing the file_name member of
struct msg_locator from "const char *" to "char *", it also touches up
places where this caused new warnings.

diff --git a/src/language/data-io/data-reader.c b/src/language/data-io/data-reader.c
index 6f620a6a948341a0dfa5db150763d20f51e92284..74e4c5692b1b83a95987bc1e16e7b92aaeebc500 100644 (file)
--- a/src/language/data-io/data-reader.c
+++ b/src/language/data-io/data-reader.c
@@ -34,6 +34,7 @@
 #include <language/lexer/lexer.h>
 #include <language/prompt.h>
 #include <libpspp/assertion.h>
+#include <libpspp/cast.h>
 #include <libpspp/integer-format.h>
 #include <libpspp/message.h>
 #include <libpspp/str.h>
@@ -140,7 +141,7 @@ dfm_open_reader (struct file_handle *fh, struct lexer *lexer)
   if (fh_get_referent (fh) != FH_REF_INLINE)
     {
       struct stat s;
-      r->where.file_name = fh_get_file_name (fh);
+      r->where.file_name = CONST_CAST (char *, fh_get_file_name (fh));
       r->where.line_number = 0;
       r->file = fn_open (fh_get_file_name (fh),
                          fh_get_mode (fh) == FH_MODE_TEXT ? "r" : "rb");

diff --git a/src/libpspp/message.c b/src/libpspp/message.c
index 5faee1f1d6c8dc33279e36acc9b3188b56b1f218..2cd0bff5911ee915c7545503e18f112b4cfd81e1 100644 (file)
--- a/src/libpspp/message.c
+++ b/src/libpspp/message.c
@@ -79,19 +79,27 @@ msg_done (void)
 struct msg *
 msg_dup(const struct msg *m)
 {
-  struct msg *new_msg = xmalloc (sizeof *m);
+  struct msg *new_msg;
 
-  *new_msg = *m;
-  new_msg->text = xstrdup(m->text);
+  new_msg = xmemdup (m, sizeof *m);
+  if (m->where.file_name != NULL)
+    new_msg->where.file_name = xstrdup (m->where.file_name);
+  new_msg->text = xstrdup (m->text);
 
   return new_msg;
 }
 
+/* Frees a message created by msg_dup().
+
+   (Messages not created by msg_dup(), as well as their where.file_name
+   members, are typically not dynamically allocated, so this function should
+   not be used to destroy them.) */
 void
-msg_destroy(struct msg *m)
+msg_destroy (struct msg *m)
 {
-  free(m->text);
-  free(m);
+  free (m->where.file_name);
+  free (m->text);
+  free (m);
 }
 
 /* Emits M as an error message.

diff --git a/src/libpspp/message.h b/src/libpspp/message.h
index be4ba3e21dcba2f25e68aa98a3f1674d97dc13d9..3aca77891e9e0df2f25e96be3317cffd63ee82e8 100644 (file)
--- a/src/libpspp/message.h
+++ b/src/libpspp/message.h
@@ -69,8 +69,8 @@ msg_class_from_category_and_severity (enum msg_category category,
 /* A file location.  */
 struct msg_locator
   {
-    const char *file_name;             /* File name. */
-    int line_number;                   /* Line number. */
+    char *file_name;           /* File name. */
+    int line_number;            /* Line number. */
   };
 
 /* A message. */

diff --git a/src/libpspp/msg-locator.c b/src/libpspp/msg-locator.c
index 09550e29ac9c802def722eb8269d50efb9ab726d..fb8082e9217f6d7d4614b35e9a347761e8ad356a 100644 (file)
--- a/src/libpspp/msg-locator.c
+++ b/src/libpspp/msg-locator.c
@@ -17,8 +17,9 @@
 #include <config.h>
 #include <stdlib.h>
 #include "msg-locator.h"
-#include <libpspp/message.h>
 #include <libpspp/assertion.h>
+#include <libpspp/cast.h>
+#include <libpspp/message.h>
 #include "getl.h"
 
 #include "xalloc.h"
@@ -77,7 +78,7 @@ get_msg_location (const struct source_stream *ss, struct msg_locator *loc)
     }
   else
     {
-      loc->file_name = getl_source_name (ss);
+      loc->file_name = CONST_CAST (char *, getl_source_name (ss));
       loc->line_number = getl_source_location (ss);
     }
 }