Re: branch-1_4 and frozen files

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> 
> Ported to head in a different manner.  I created a new symbol type that
> tracks the name of the requested builtin.  This could open the door to
> future enhancements that a placeholder can be replaced with an actual
> function by properly loading a newer module.  With this patch, the
> testsuite once again passes all tests (which is a sign that it is
> incomplete, because I'm not done porting from the branch).

And testing showed that I had a memory bug when the placeholder was actually 
dereferenced.

2006-07-28  Eric Blake  <address@hidden>

        * src/freeze.c (reload_frozen_state): Copy string when creating
        placeholder, to avoid memory corruption.
        * m4/symtab.c (symbol_popval): Avoid memory leak.
        (m4_symbol_rename): Avoid shadowing rename function.
        (dump_symbol_CB, symtab_dump) [DEBUG_SYM]: Fix compilation.
        * tests/freeze.at (reloading unknown builtin): Add test.

Index: src/freeze.c
===================================================================
RCS file: /sources/m4/m4/src/freeze.c,v
retrieving revision 1.46
diff -u -r1.46 freeze.c
--- src/freeze.c        28 Jul 2006 14:06:11 -0000      1.46
+++ src/freeze.c        28 Jul 2006 15:50:28 -0000
@@ -558,7 +558,7 @@
              }
            else
              {
-               m4_set_symbol_value_placeholder (token, string[1]);
+               m4_set_symbol_value_placeholder (token, xstrdup (string[1]));
                VALUE_MIN_ARGS (token) = -1;
                VALUE_MAX_ARGS (token) = -1;
              }
Index: m4/symtab.c
===================================================================
RCS file: /sources/m4/m4/m4/symtab.c,v
retrieving revision 1.55
diff -u -r1.55 symtab.c
--- m4/symtab.c 28 Jul 2006 14:06:11 -0000      1.55
+++ m4/symtab.c 28 Jul 2006 15:50:28 -0000
@@ -305,19 +305,22 @@
        }
       if (m4_is_symbol_value_text (stale))
        free (m4_get_symbol_value_text (stale));
+      else if (m4_is_symbol_value_placeholder (stale))
+       free (m4_get_symbol_value_placeholder (stale));
       free (stale);
     }
 }
 
 m4_symbol *
-m4_symbol_rename (m4_symbol_table *symtab, const char *name, const char 
*rename)
+m4_symbol_rename (m4_symbol_table *symtab, const char *name,
+                 const char *newname)
 {
   m4_symbol *symbol    = NULL;
   m4_symbol **psymbol;
 
   assert (symtab);
   assert (name);
-  assert (rename);
+  assert (newname);
 
   /* Use a low level hash fetch, so we can save the symbol value when
      removing the symbol name from the symbol table.  */
@@ -331,7 +334,7 @@
       free (m4_hash_remove (symtab->table, name));
       assert (!m4_hash_lookup (symtab->table, name));
 
-      m4_hash_insert (symtab->table, xstrdup (rename), *psymbol);
+      m4_hash_insert (symtab->table, xstrdup (newname), *psymbol);
     }
   /* else
        NAME does not name a symbol in symtab->table!  */
@@ -548,13 +551,12 @@
 
 #ifdef DEBUG_SYM
 
-static void *symtab_dump       (m4_symbol_table *symtab);
-static void  dump_symbol_CB    (m4_symbol_table *symtab, const char *name,
+static void *dump_symbol_CB    (m4_symbol_table *symtab, const char *name,
                                 m4_symbol *symbol, void *userdata);
-static void *
+static M4_GNUC_UNUSED void *
 symtab_dump (m4_symbol_table *symtab)
 {
-  return symtab_apply (symtab, dump_symbol_CB, NULL);
+  return m4_symtab_apply (symtab, dump_symbol_CB, NULL);
 }
 
 static void *
@@ -567,9 +569,7 @@
   const char *     module_name = handle ? m4_get_module_name (handle) : "NONE";
   const m4_builtin *bp;
 
-  fprintf (stderr, "%10s: (%d%s) %s=",
-          handle ? m4_get_module_name (handle) : "NONE",
-          value  ? VALUE_FLAGS (value) : 0,
+  fprintf (stderr, "%10s: (%d%s) %s=", module_name, flags,
           m4_get_symbol_traced (symbol) ? "!" : "", name);
 
   if (!value)
@@ -595,5 +595,6 @@
        break;
       }
   fputc ('\n', stderr);
+  return NULL;
 }
 #endif /* DEBUG_SYM */
Index: tests/freeze.at
===================================================================
RCS file: /sources/m4/m4/tests/freeze.at,v
retrieving revision 1.3
diff -u -r1.3 freeze.at
--- tests/freeze.at     28 Jul 2006 14:06:11 -0000      1.3
+++ tests/freeze.at     28 Jul 2006 15:50:28 -0000
@@ -106,6 +106,53 @@
 AT_CLEANUP
 
 
+## -------------------------- ##
+## Reloading unknown builtins ##
+## -------------------------- ##
+
+AT_SETUP([reloading unknown builtin])
+AT_KEYWORDS([frozen])
+
+AT_DATA([[empty.m4]])
+
+# Freeze default state.
+AT_CHECK_M4([-F frozen.m4f empty.m4])
+
+# Add an unknown builtin.
+echo 'F1,1' >> frozen.m4f
+echo 'ab' >> frozen.m4f
+
+AT_DATA([[input.m4]],
+[[dnl The macro is defined; checking this is safe
+ifdef(`a', `yes', `no')
+dnl Grabbing the definition must warn
+defn(`a')
+dnl Invoking the macro directly must warn
+a
+dnl Invoking it indirectly must warn
+indir(`a')
+dnl Since it is defined, it must have a definition
+dumpdef(`a')
+dnl Deleting it is safe
+popdef(`a')
+]])
+
+AT_CHECK_M4([-R frozen.m4f input.m4], 0,
+[[yes
+
+
+
+
+
+]],
+[[m4: input.m4: 4: Warning: a: builtin `b' requested by frozen file not found
+m4: input.m4: 6: Warning: a: builtin `b' requested by frozen file not found
+m4: input.m4: 8: Warning: a: builtin `b' requested by frozen file not found
+a:     <placeholder for b>
+]])
+
+AT_CLEANUP
+
 ## ---------------------- ##
 ## Freezing regexp syntax ##
 ## ---------------------- ##