Sanitize the registered function interface.

Expand the characters which are legal in a function name, and check
the name for validity.  Create a type for the function pointer.
Convert the last argument from a boolean to flags, to allow for expansion.

11 files changed, 143 insertions, 49 deletions

diff --git a/ChangeLog b/ChangeLog
index 7d92dcf..19165ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,26 @@
+2013-10-05  Paul Smith  <psmith@gnu.org>
+
+	* makeint.h (MAP_USERFUNC): A new map type for function names.
+	* main.c (initialize_stopchar_map): Set up the function name map.
+
+	* gnumake.h (gmk_func_ptr): Define a type for function pointers.
+	(gmk_add_function): Convert the last argument to FLAGS.
+	(GMK_FUNC_*): Define flags for the function.  Change the default
+	behavior to "expand" since that's the most common one.
+
+	* function.c (function_table_entry): Use new function pointer type.
+	(lookup_function): Accept any valid function name character based
+	on the MAP_USERFUNC values.
+	(define_new_function): Use the new calling signature.  Verify that
+	registered functions have valid names.
+
+	* guile.c (guile_gmake_setup): Use new calling signatures.
+	* loadapi.c (gmk_add_function): Ditto.
+	* variable.h (define_new_function): Ditto.
+
+	* doc/make.texi (Loaded Object API): Make the registered function
+	API documentation more clear.
+
 2013-10-03  Eli Zaretskii  <eliz@gnu.org>
 
 	* function.c (abspath): Reset root_len to one for Cygwin only when
diff --git a/doc/make.texi b/doc/make.texi
index 42cec7f..8fbdb61 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -11115,16 +11115,14 @@ arguments are as follows:
 @table @code
 @item name
 The function name.  This is what the makefile should use to invoke the
-function.  The name must be between 1 and 255 characters long.
+function.  The name must be between 1 and 255 characters long and it
+may only contain alphanumeric, period (@samp{.}), dash (@samp{-}), and
+underscore (@samp{_}) characters.  It may not begin with a period.
 
 @item func_ptr
 A pointer to a function that @code{make} will invoke when it expands
 the function in a makefile.  This function must be defined by the
-loaded object.  GNU @code{make} will call it with three arguments:
-@code{name} (the same name as above), @code{argc} (the number of
-arguments to the function), and @code{argv} (the list of arguments to
-the function).  The last argument (that is, @code{argv[argc]} will be
-null (@code{0}).
+loaded object.
 
 @item min_args
 The minimum number of arguments the function will accept.  Must be
@@ -11140,12 +11138,30 @@ arguments.  If the value is 0, then any number of arguments is
 accepted.  If the value is greater than 0, then it must be greater
 than or equal to @code{min_args}.
 
-@item expand_args
-If this value is 0, then @code{make} will not expand the arguments to
-the function before passing them to @code{func_ptr}.  If the value is
-non-0, then the arguments will be expanded first.
+@item flags
+Flags that specify how this function will operate; the desired flags
+should be OR'd together.  If the @code{GMK_FUNC_NOEXPAND} flag is
+given then the function arguments will not be expanded before the
+function is called; otherwise they will be expanded first.
 @end table
 
+@subsubheading Registered Function Interface
+@findex gmk_func_ptr
+
+A function registered with @code{make} must match the
+@code{gmk_func_ptr} type.  It will be invoked with three parameters:
+@code{name} (the name of the function), @code{argc} (the number of
+arguments to the function), and @code{argv} (an array of pointers to
+arguments to the function).  The last pointer (that is,
+@code{argv[argc]}) will be null (@code{0}).
+
+The return value of the function is the result of expanding the
+function.  If the function expands to nothing the return value may be
+null.  Otherwise, it must be a pointer to a string created with
+@code{gmk_alloc}.  Once the function returns, @code{make} owns this
+string and will free it when appropriate; it cannot be accessed by the
+loaded object.
+
 @subsubheading GNU @code{make} Facilities
 
 There are some facilities exported by GNU @code{make} for use by
@@ -11184,6 +11200,12 @@ should never pass memory that you've allocated directly to any
 memory returned to you by any @code{make} function.  Instead, use the
 @code{gmk_alloc} and @code{gmk_free} functions.
 
+In particular, the string returned to @code{make} by a function
+registered using @code{gmk_add_function} @emph{must} be allocated
+using @code{gmk_alloc}, and the string returned from the @code{make}
+@code{gmk_expand} function @emph{must} be freed (when no longer
+needed) using @code{gmk_free}.
+
 @table @code
 @item gmk_alloc
 @findex gmk_alloc
diff --git a/function.c b/function.c
index 36e0268..ecce627 100644
--- a/function.c
+++ b/function.c
@@ -31,7 +31,7 @@ struct function_table_entry
   {
     union {
       char *(*func_ptr) (char *output, char **argv, const char *fname);
-      char *(*alloc_func_ptr) (const char *fname, int argc, char **argv);
+      gmk_func_ptr alloc_func_ptr;
     } fptr;
     const char *name;
     unsigned char len;
@@ -269,19 +269,19 @@ patsubst_expand (char *o, const char *text, char *pattern, char *replace)
 static const struct function_table_entry *
 lookup_function (const char *s)
 {
+  struct function_table_entry function_table_entry_key;
   const char *e = s;
 
-  while (*e && ( (*e >= 'a' && *e <= 'z') || *e == '-'))
+  while (STOP_SET (*e, MAP_USERFUNC))
     e++;
-  if (*e == '\0' || isblank ((unsigned char) *e))
-    {
-      struct function_table_entry function_table_entry_key;
-      function_table_entry_key.name = s;
-      function_table_entry_key.len = e - s;
 
-      return hash_find_item (&function_table, &function_table_entry_key);
-    }
-  return 0;
+  if (e == s || !STOP_SET(*e, MAP_NUL|MAP_SPACE))
+    return NULL;
+
+  function_table_entry_key.name = s;
+  function_table_entry_key.len = e - s;
+
+  return hash_find_item (&function_table, &function_table_entry_key);
 }
 
 
@@ -2445,7 +2445,7 @@ func_call (char *o, char **argv, const char *funcname UNUSED)
   /* There is no way to define a variable with a space in the name, so strip
      leading and trailing whitespace as a favor to the user.  */
   fname = argv[0];
-  while (*fname != '\0' && isspace ((unsigned char)*fname))
+  while (isspace ((unsigned char)*fname))
     ++fname;
 
   cp = fname + strlen (fname) - 1;
@@ -2530,19 +2530,28 @@ func_call (char *o, char **argv, const char *funcname UNUSED)
 }
 
 void
-define_new_function (const gmk_floc *flocp,
-                     const char *name, int min, int max, int expand,
-                     char *(*func)(const char *, int, char **))
+define_new_function (const gmk_floc *flocp, const char *name,
+                     unsigned int min, unsigned int max, unsigned int flags,
+                     gmk_func_ptr func)
 {
+  const char *e = name;
   struct function_table_entry *ent;
-  size_t len = strlen (name);
+  size_t len;
+
+  while (STOP_SET (*e, MAP_USERFUNC))
+    e++;
+  len = e - name;
 
+  if (len == 0)
+    fatal (flocp, _("Empty function name\n"));
+  if (*name == '.' || *e != '\0')
+    fatal (flocp, _("Invalid function name: %s\n"), name);
   if (len > 255)
     fatal (flocp, _("Function name too long: %s\n"), name);
-  if (min < 0 || min > 255)
+  if (min > 255)
     fatal (flocp, _("Invalid minimum argument count (%d) for function %s\n"),
            min, name);
-  if (max < 0 || max > 255 || (max && max < min))
+  if (max > 255 || (max && max < min))
     fatal (flocp, _("Invalid maximum argument count (%d) for function %s\n"),
            max, name);
 
@@ -2551,7 +2560,7 @@ define_new_function (const gmk_floc *flocp,
   ent->len = len;
   ent->minimum_args = min;
   ent->maximum_args = max;
-  ent->expand_args = expand ? 1 : 0;
+  ent->expand_args = ANY_SET(flags, GMK_FUNC_NOEXPAND) ? 0 : 1;
   ent->alloc_fn = 1;
   ent->fptr.alloc_func_ptr = func;
 
diff --git a/gnumake.h b/gnumake.h
index dbf303b..94d725e 100644
--- a/gnumake.h
+++ b/gnumake.h
@@ -26,6 +26,7 @@ typedef struct
     unsigned long lineno;
   } gmk_floc;
 
+typedef char *(*gmk_func_ptr)(const char *nm, unsigned int argc, char **argv);
 
 #ifdef _WIN32
 # ifdef GMK_BUILDING_MAKE
@@ -60,14 +61,19 @@ GMK_EXPORT char *gmk_expand (const char *str);
 
    MIN_ARGS is the minimum number of arguments the function requires.
    MAX_ARGS is the maximum number of arguments (or 0 if there's no maximum).
-   MIN_ARGS and MAX_ARGS must be >= 0 and <= 255.
+   MIN_ARGS and MAX_ARGS may not exceed 255.
 
-   If EXPAND_ARGS is 0, the arguments to the function will not be expanded
-   before FUNC is called.  If EXPAND_ARGS is non-0, they will be expanded.
+   The FLAGS value may be GMK_FUNC_DEFAULT, or one or more of the following
+   flags OR'd together:
+
+     GMK_FUNC_NOEXPAND: the arguments to the function will be not be expanded
+                        before FUNC is called.
 */
-GMK_EXPORT void gmk_add_function (const char *name,
-                                  char *(*func)(const char *nm,
-                                                int argc, char **argv),
-                                  int min_args, int max_args, int expand_args);
+GMK_EXPORT void gmk_add_function (const char *name, gmk_func_ptr func,
+                                  unsigned int min_args, unsigned int max_args,
+                                  unsigned int flags);
+
+#define GMK_FUNC_DEFAULT    0x00
+#define GMK_FUNC_NOEXPAND   0x01
 
 #endif  /* _GNUMAKE_H_ */
diff --git a/guile.c b/guile.c
index 07a2147..499585c 100644
--- a/guile.c
+++ b/guile.c
@@ -115,7 +115,7 @@ internal_guile_eval (void *arg)
 
 /* This is the function registered with make  */
 static char *
-func_guile (const char *funcname UNUSED, int argc UNUSED, char **argv)
+func_guile (const char *funcname UNUSED, unsigned int argc UNUSED, char **argv)
 {
   static int init = 0;
 
@@ -140,7 +140,7 @@ int
 guile_gmake_setup (const gmk_floc *flocp UNUSED)
 {
   /* Create a make function "guile".  */
-  gmk_add_function ("guile", func_guile, 0, 1, 1);
+  gmk_add_function ("guile", func_guile, 0, 1, GMK_FUNC_DEFAULT);
 
   return 1;
 }
diff --git a/loadapi.c b/loadapi.c
index e314390..0568cbd 100644
--- a/loadapi.c
+++ b/loadapi.c
@@ -54,9 +54,8 @@ gmk_expand (const char *ref)
 
 /* Register a function to be called from makefiles.  */
 void
-gmk_add_function (const char *name,
-                  char *(*func)(const char *nm, int argc, char **argv),
-                  int min, int max, int expand)
+gmk_add_function (const char *name, gmk_func_ptr func,
+                  unsigned int min, unsigned int max, unsigned int flags)
 {
-  define_new_function (reading_file, name, min, max, expand, func);
+  define_new_function (reading_file, name, min, max, flags, func);
 }
diff --git a/main.c b/main.c
index 7069669..f60e6be 100644
--- a/main.c
+++ b/main.c
@@ -612,10 +612,13 @@ initialize_stopchar_map ()
   stopchar_map[(int)':'] = MAP_COLON;
   stopchar_map[(int)'%'] = MAP_PERCENT;
   stopchar_map[(int)'|'] = MAP_PIPE;
-  stopchar_map[(int)'.'] = MAP_DOT;
+  stopchar_map[(int)'.'] = MAP_DOT | MAP_USERFUNC;
   stopchar_map[(int)','] = MAP_COMMA;
   stopchar_map[(int)'$'] = MAP_VARIABLE;
 
+  stopchar_map[(int)'-'] = MAP_USERFUNC;
+  stopchar_map[(int)'_'] = MAP_USERFUNC;
+
   stopchar_map[(int)'/'] = MAP_PATHSEP;
 #if defined(VMS)
   stopchar_map[(int)']'] = MAP_PATHSEP;
@@ -629,6 +632,8 @@ initialize_stopchar_map ()
         stopchar_map[i] = MAP_BLANK;
       if (isspace(i))
         stopchar_map[i] |= MAP_SPACE;
+      if (isalnum(i))
+        stopchar_map[i] = MAP_USERFUNC;
     }
 }
 
diff --git a/makeint.h b/makeint.h
index f48c087..3e22296 100644
--- a/makeint.h
+++ b/makeint.h
@@ -396,6 +396,8 @@ extern int unixy_shell;
 #define MAP_DOT         0x0200
 #define MAP_COMMA       0x0400
 
+/* These are the valid characters for a user-defined function.  */
+#define MAP_USERFUNC    0x2000
 /* This means not only a '$', but skip the variable reference.  */
 #define MAP_VARIABLE    0x4000
 /* The set of characters which are path separators is OS-specific.  */
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 75cf8dc..0ff5033 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2013-10-05  Paul Smith  <psmith@gnu.org>
+
+	* scripts/features/loadapi: Use new calling signatures.  Verify
+	the NOEXPAND flag works.  Test with all valid function name
+	characters.
+
 2013-09-29  Paul Smith  <psmith@gnu.org>
 
 	* scripts/variables/SHELL: Solaris /bin/sh can't handle options in
diff --git a/tests/scripts/features/loadapi b/tests/scripts/features/loadapi
index 4976ce3..6d3b03f 100644
--- a/tests/scripts/features/loadapi
+++ b/tests/scripts/features/loadapi
@@ -36,7 +36,15 @@ test_expand (const char *val)
 }
 
 static char *
-func_test (const char *funcname, int argc, char **argv)
+test_noexpand (const char *val)
+{
+    char *str = gmk_alloc (strlen (val));
+    strcpy (str, val);
+    return str;
+}
+
+static char *
+func_test (const char *funcname, unsigned int argc, char **argv)
 {
     char *mem;
 
@@ -46,7 +54,10 @@ func_test (const char *funcname, int argc, char **argv)
     if (strcmp (funcname, "test-eval") == 0)
         return test_eval (argv[0]);
 
-    mem = gmk_alloc (strlen ("unknown") + 1);
+    if (strcmp (funcname, "test-noexpand") == 0)
+        return test_noexpand (argv[0]);
+
+    mem = gmk_alloc (sizeof ("unknown"));
     strcpy (mem, "unknown");
     return mem;
 }
@@ -54,8 +65,10 @@ func_test (const char *funcname, int argc, char **argv)
 int
 testapi_gmk_setup ()
 {
-    gmk_add_function ("test-expand", func_test, 1, 1, 1);
-    gmk_add_function ("test-eval", func_test, 1, 1, 1);
+    gmk_add_function ("test-expand", func_test, 1, 1, GMK_FUNC_DEFAULT);
+    gmk_add_function ("test-noexpand", func_test, 1, 1, GMK_FUNC_NOEXPAND);
+    gmk_add_function ("test-eval", func_test, 1, 1, GMK_FUNC_DEFAULT);
+    gmk_add_function ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.", func_test, 0, 0, 0);
     return 1;
 }
 EOF
@@ -84,6 +97,15 @@ all:;@echo '$(VAR)'
 !,
               '', "hi there\n");
 
+# TEST 2
+# Check the no-expand capability
+run_make_test(q!
+load testapi.so
+TEST = hi
+all:;@echo '$(test-noexpand $(TEST))'
+!,
+              '', "\$(TEST)\n");
+
 unlink(qw(testapi.c testapi.so)) unless $keep;
 
 # This tells the test driver that the perl test script executed properly.
diff --git a/variable.h b/variable.h
index 65864e4..dec75b1 100644
--- a/variable.h
+++ b/variable.h
@@ -165,9 +165,9 @@ struct variable *try_variable_definition (const gmk_floc *flocp, char *line,
                                           int target_var);
 void init_hash_global_variable_set (void);
 void hash_init_function_table (void);
-void define_new_function(const gmk_floc *flocp,
-                         const char *name, int min, int max, int expand,
-                         char *(*func)(const char *, int, char **));
+void define_new_function(const gmk_floc *flocp, const char *name,
+                         unsigned int min, unsigned int max, unsigned int flags,
+                         gmk_func_ptr func);
 struct variable *lookup_variable (const char *name, unsigned int length);
 struct variable *lookup_variable_in_set (const char *name, unsigned int length,
                                          const struct variable_set *set);