From 2fb91e19a02ff8a3e43145d582cfebaf3a60f1d9 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 5 Oct 2013 16:10:30 -0400 Subject: [PATCH] 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. --- ChangeLog | 23 +++++++++++++++++ doc/make.texi | 42 +++++++++++++++++++++++-------- function.c | 45 ++++++++++++++++++++-------------- gnumake.h | 20 +++++++++------ guile.c | 4 +-- loadapi.c | 7 +++--- main.c | 7 +++++- makeint.h | 2 ++ tests/ChangeLog | 6 +++++ tests/scripts/features/loadapi | 30 ++++++++++++++++++++--- variable.h | 6 ++--- 11 files changed, 143 insertions(+), 49 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7d92dcfc..19165ac4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2013-10-05 Paul Smith + + * 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 * function.c (abspath): Reset root_len to one for Cygwin only when diff --git a/doc/make.texi b/doc/make.texi index 42cec7fe..8fbdb614 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 36e0268a..ecce627f 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 dbf303b9..94d725e8 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 07a21478..499585cf 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 e314390b..0568cbd4 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 7069669a..f60e6be9 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 f48c0873..3e222961 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 75cf8dcf..0ff50339 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,9 @@ +2013-10-05 Paul Smith + + * scripts/features/loadapi: Use new calling signatures. Verify + the NOEXPAND flag works. Test with all valid function name + characters. + 2013-09-29 Paul Smith * scripts/variables/SHELL: Solaris /bin/sh can't handle options in diff --git a/tests/scripts/features/loadapi b/tests/scripts/features/loadapi index 4976ce3b..6d3b03f7 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 65864e4f..dec75b1c 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);