From: Ben Pfaff <blp@cs.stanford.edu>
Date: Sat, 3 Jul 2021 18:46:17 +0000 (-0700)
Subject: message: Make msg_emit() take full ownership of its argument.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9165b88a0a735f26977d9d556294d8e50efb9bfe;p=pspp

message: Make msg_emit() take full ownership of its argument.

The way it treated the argument before was just confusing.
---

diff --git a/src/data/pc+-file-reader.c b/src/data/pc+-file-reader.c
index c7c9e3b7f8..8b945788a5 100644
--- a/src/data/pc+-file-reader.c
+++ b/src/data/pc+-file-reader.c
@@ -1159,12 +1159,13 @@ pcp_msg (struct pcp_reader *r, off_t offset,
     ds_put_format (&text, _("`%s': "), fh_get_file_name (r->fh));
   ds_put_vformat (&text, format, args);
 
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = msg_class_to_category (class),
     .severity = msg_class_to_severity (class),
-    .text = ds_cstr (&text),
+    .text = ds_steal_cstr (&text),
   };
-  msg_emit (&m);
+  msg_emit (m);
 }
 
 /* Displays a warning for offset OFFSET in the file. */
diff --git a/src/data/por-file-reader.c b/src/data/por-file-reader.c
index 343615a826..8d008b1dd6 100644
--- a/src/data/por-file-reader.c
+++ b/src/data/por-file-reader.c
@@ -112,12 +112,13 @@ error (struct pfm_reader *r, const char *msg, ...)
   ds_put_vformat (&text, msg, args);
   va_end (args);
 
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = MSG_C_GENERAL,
     .severity = MSG_S_ERROR,
-    .text = ds_cstr (&text),
+    .text = ds_steal_cstr (&text),
   };
-  msg_emit (&m);
+  msg_emit (m);
 
   r->ok = false;
 
@@ -139,12 +140,13 @@ warning (struct pfm_reader *r, const char *msg, ...)
   ds_put_vformat (&text, msg, args);
   va_end (args);
 
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = MSG_C_GENERAL,
     .severity = MSG_S_WARNING,
-    .text = ds_cstr (&text),
+    .text = ds_steal_cstr (&text),
   };
-  msg_emit (&m);
+  msg_emit (m);
 }
 
 /* Close and destroy R.
diff --git a/src/data/sys-file-reader.c b/src/data/sys-file-reader.c
index bc5ff9eeb2..f11480ac8f 100644
--- a/src/data/sys-file-reader.c
+++ b/src/data/sys-file-reader.c
@@ -3244,12 +3244,13 @@ sys_msg (struct sfm_reader *r, off_t offset,
     ds_put_format (&text, _("`%s': "), fh_get_file_name (r->fh));
   ds_put_vformat (&text, format, args);
 
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = msg_class_to_category (class),
     .severity = msg_class_to_severity (class),
-    .text = ds_cstr (&text),
+    .text = ds_steal_cstr (&text),
   };
-  msg_emit (&m);
+  msg_emit (m);
 }
 
 /* Displays a warning for offset OFFSET in the file. */
diff --git a/src/language/control/define.c b/src/language/control/define.c
index e8155e9214..0c227233c4 100644
--- a/src/language/control/define.c
+++ b/src/language/control/define.c
@@ -81,7 +81,11 @@ cmd_define (struct lexer *lexer, struct dataset *ds UNUSED)
 
   /* Parse macro name. */
   struct macro *m = xmalloc (sizeof *m);
-  *m = (struct macro) { .name = ss_xstrdup (lex_tokss (lexer)) };
+  *m = (struct macro) {
+    .name = ss_xstrdup (lex_tokss (lexer)),
+    .file_name = xstrdup_if_nonnull (lex_get_file_name (lexer)),
+    .first_line = lex_get_first_line_number (lexer, 0),
+  };
   lex_get (lexer);
 
   if (!lex_force_match (lexer, T_LPAREN))
@@ -227,6 +231,7 @@ cmd_define (struct lexer *lexer, struct dataset *ds UNUSED)
       ds_put_byte (&body, '\n');
       lex_get (lexer);
     }
+  m->last_line = lex_get_last_line_number (lexer, 0);
 
   macro_tokens_from_string (&m->body, body.ss, lex_get_syntax_mode (lexer));
   ds_destroy (&body);
diff --git a/src/language/data-io/data-parser.c b/src/language/data-io/data-parser.c
index 6de88b170d..03c6dbbb0e 100644
--- a/src/language/data-io/data-parser.c
+++ b/src/language/data-io/data-parser.c
@@ -496,21 +496,23 @@ parse_error (const struct dfm_reader *reader, const struct field *field,
              int first_column, int last_column, char *error)
 {
   int line_number = dfm_get_line_number (reader);
-  const struct msg_location location = {
-    .file_name = CONST_CAST (char *, dfm_get_file_name (reader)),
+  struct msg_location *location = xmalloc (sizeof *location);
+  *location = (struct msg_location) {
+    .file_name = xstrdup (dfm_get_file_name (reader)),
     .first_line = line_number,
     .last_line = line_number + 1,
     .first_column = first_column,
     .last_column = last_column,
   };
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = MSG_C_DATA,
     .severity = MSG_S_WARNING,
-    .location = CONST_CAST (struct msg_location *, &location),
+    .location = location,
     .text = xasprintf (_("Data for variable %s is not valid as format %s: %s"),
                        field->name, fmt_name (field->format.type), error),
   };
-  msg_emit (&m);
+  msg_emit (m);
 
   free (error);
 }
diff --git a/src/language/lexer/lexer.c b/src/language/lexer/lexer.c
index 9b55a4edbf..67ecfee07d 100644
--- a/src/language/lexer/lexer.c
+++ b/src/language/lexer/lexer.c
@@ -1588,20 +1588,22 @@ lex_source_error_valist (struct lex_source *src, int n0, int n1,
   if (ds_last (&s) != '.')
     ds_put_byte (&s, '.');
 
-  struct msg_location location = {
-    .file_name = src->reader->file_name,
+  struct msg_location *location = xmalloc (sizeof *location);
+  *location = (struct msg_location) {
+    .file_name = xstrdup_if_nonnull (src->reader->file_name),
     .first_line = lex_source_get_first_line_number (src, n0),
     .last_line = lex_source_get_last_line_number (src, n1),
     .first_column = lex_source_get_first_column (src, n0),
     .last_column = lex_source_get_last_column (src, n1),
   };
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = MSG_C_SYNTAX,
     .severity = MSG_S_ERROR,
-    .location = &location,
+    .location = location,
     .text = ds_steal_cstr (&s),
   };
-  msg_emit (&m);
+  msg_emit (m);
 }
 
 static void PRINTF_FORMAT (2, 3)
diff --git a/src/language/lexer/macro.c b/src/language/lexer/macro.c
index 5f68de3fdd..ad6a2be20e 100644
--- a/src/language/lexer/macro.c
+++ b/src/language/lexer/macro.c
@@ -328,6 +328,7 @@ macro_destroy (struct macro *m)
     return;
 
   free (m->name);
+  free (m->file_name);
   for (size_t i = 0; i < m->n_params; i++)
     {
       struct macro_param *p = &m->params[i];
@@ -427,6 +428,9 @@ struct macro_expansion_stack
   {
     const struct macro_expansion_stack *next;
     const char *name;
+    const char *file_name;
+    int first_line;
+    int last_line;
   };
 
 
@@ -1539,16 +1543,45 @@ macro_error (const struct macro_expansion_stack *stack,
   char *s = xvasprintf (format, args);
   va_end (args);
 
-  /* foo.sps:12: While expanding macro 'innermost',
+  /* foo.sps:12: While expanding 'innermost',
      foo.sps:23: inside expansion of 'next_inner',
      foo.sps:34: inside expansion of 'next_inner2',
      foo.sps:45: inside expansion of 'outermost':
      error. */
   struct string header = DS_EMPTY_INITIALIZER;
-  ds_put_format (&header, "While expanding \"%s\"", stack->name);
+  if (stack->file_name || stack->first_line)
+    {
+      if (stack->file_name)
+        ds_put_format (&header, "%s:", stack->file_name);
+      if (stack->first_line)
+        {
+          if (stack->last_line > stack->first_line)
+            ds_put_format (&header, "%d-%d:",
+                           stack->first_line, stack->last_line);
+          else
+            ds_put_format (&header, "%d", stack->first_line);
+        }
+      ds_put_byte (&header, ' ');
+    }
+  ds_put_format (&header, "While expanding \"%s\"\n", stack->name);
   while ((stack = stack->next) != NULL)
-    ds_put_format (&header, ", inside expansion of \"%s\"", stack->name);
-
+    {
+      if (stack->file_name || stack->first_line)
+        {
+          if (stack->file_name)
+            ds_put_format (&header, "%s:", stack->file_name);
+          if (stack->first_line)
+            {
+              if (stack->last_line > stack->first_line)
+                ds_put_format (&header, "%d-%d:",
+                               stack->first_line, stack->last_line);
+              else
+                ds_put_format (&header, "%d", stack->first_line);
+            }
+          ds_put_byte (&header, ' ');
+        }
+    ds_put_format (&header, ", inside expansion of \"%s\"\n", stack->name);
+    }
   msg (SE, "%s: %s", ds_cstr (&header), s);
 
   ds_destroy (&header);
@@ -1885,6 +1918,9 @@ macro_expand (const struct macro_tokens *mts,
                             subme, NULL,
                             &(struct macro_expansion_stack) {
                               .name = subme->macro->name,
+                              .file_name = subme->macro->file_name,
+                              .first_line = subme->macro->first_line,
+                              .last_line = subme->macro->last_line,
                               .next = stack,
                             }, expand, break_, exp);
               macro_expander_destroy (subme);
@@ -1967,7 +2003,12 @@ void
 macro_expander_get_expansion (struct macro_expander *me, struct macro_tokens *exp)
 {
   bool expand = true;
-  struct macro_expansion_stack stack = { .name = me->macro->name };
+  struct macro_expansion_stack stack = {
+    .name = me->macro->name,
+    .file_name = me->macro->file_name,
+    .first_line = me->macro->first_line,
+    .last_line = me->macro->last_line,
+  };
   macro_expand (&me->macro->body, settings_get_mnest (),
                 me->macros, me, NULL, &stack, &expand, NULL, exp);
 }
diff --git a/src/language/lexer/macro.h b/src/language/lexer/macro.h
index c10ce8e3cd..529eed05fd 100644
--- a/src/language/lexer/macro.h
+++ b/src/language/lexer/macro.h
@@ -88,6 +88,11 @@ struct macro
     struct hmap_node hmap_node; /* Indexed by 'name'. */
     char *name;
 
+    /* Source code location of macro definition. */
+    char *file_name;
+    int first_line;
+    int last_line;
+
     struct macro_param *params;
     size_t n_params;
 
diff --git a/src/libpspp/message.c b/src/libpspp/message.c
index 7d4f9d8430..bbba1c5506 100644
--- a/src/libpspp/message.c
+++ b/src/libpspp/message.c
@@ -53,13 +53,13 @@ static int messages_disabled;
 void
 vmsg (enum msg_class class, const char *format, va_list args)
 {
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = msg_class_to_category (class),
     .severity = msg_class_to_severity (class),
     .text = xvasprintf (format, args),
   };
-
-  msg_emit (&m);
+  msg_emit (m);
 }
 
 /* Writes error message in CLASS, with text FORMAT, formatted with
@@ -80,17 +80,18 @@ msg_error (int errnum, const char *format, ...)
 {
   va_list args;
   va_start (args, format);
-  char *e = xvasprintf (format, args);
+  struct string s = DS_EMPTY_INITIALIZER;
+  ds_put_vformat (&s, format, args);
   va_end (args);
+  ds_put_format (&s, ": %s", strerror (errnum));
 
-  struct msg m = {
+  struct msg *m = xmalloc (sizeof *m);
+  *m = (struct msg) {
     .category = MSG_C_GENERAL,
     .severity = MSG_S_ERROR,
-    .text = xasprintf (_("%s: %s"), e, strerror (errnum)),
+    .text = ds_steal_cstr (&s),
   };
-  msg_emit (&m);
-
-  free (e);
+  msg_emit (m);
 }
 
 
@@ -315,10 +316,10 @@ msg_ui_any_errors (void)
 
 
 static void
-ship_message (struct msg *m)
+ship_message (const struct msg *m)
 {
   enum { MAX_STACK = 4 };
-  static struct msg *stack[MAX_STACK];
+  static const struct msg *stack[MAX_STACK];
   static size_t n;
 
   /* If we're recursing on a given message, or recursing deeply, drop it. */
@@ -349,8 +350,6 @@ submit_note (char *s)
   free (s);
 }
 
-
-
 static void
 process_msg (struct msg *m)
 {
@@ -391,16 +390,13 @@ process_msg (struct msg *m)
 }
 
 
-/* Emits M as an error message.
-   Frees allocated data in M. */
+/* Emits M as an error message.  Takes ownership of M. */
 void
 msg_emit (struct msg *m)
 {
   if (!messages_disabled)
-     process_msg (m);
-
-  free (m->text);
-  free (m->command_name);
+    process_msg (m);
+  msg_destroy (m);
 }
 
 /* Disables message output until the next call to msg_enable.  If