diff --git a/libguile/ChangeLog b/libguile/ChangeLog index dfb6d67df..5295163d2 100644 --- a/libguile/ChangeLog +++ b/libguile/ChangeLog @@ -1,3 +1,15 @@ +2002-06-30 Gary Houston + + * posix.c (scm_convert_exec_args), dynl.c + (scm_make_argv_from_stringlist): static procs: 1) renamed both to + allocate_string_pointers. 2) simplified: don't reallocate the + strings, just make an array of pointers 3) avoid memory leaks on + error 4) let the procedure report errors in its own name. + Consequences: 1) the procedures now assume that SCM strings are + nul-terminated, which should always be the case. 2) Since strings + are not reallocated, it's now possible for strings passed to + dynamic-args-call to be mutated. + 2002-06-28 Dirk Herrmann * __scm.h, eval.c, eval.h: Removed compile time option diff --git a/libguile/dynl.c b/libguile/dynl.c index 1f8c3ca70..2f8eac161 100644 --- a/libguile/dynl.c +++ b/libguile/dynl.c @@ -75,60 +75,6 @@ maybe_drag_in_eprintf () #include "libguile/lang.h" #include "libguile/validate.h" -/* Create a new C argv array from a scheme list of strings. */ -/* Dirk:FIXME:: A quite similar function is implemented in posix.c */ -/* Dirk:FIXME:: In case of assertion errors, we get memory leaks */ - -/* Converting a list of SCM strings into a argv-style array. You must - have ints disabled for the whole lifetime of the created argv (from - before MAKE_ARGV_FROM_STRINGLIST until after - MUST_FREE_ARGV). Atleast this is was the documentation for - MAKARGVFROMSTRS says, it isn't really used that way. - - This code probably belongs into strings.c - (Dirk: IMO strings.c is not the right place.) */ - -static char ** -scm_make_argv_from_stringlist (SCM args, int *argcp, const char *subr, - int argn) -{ - char **argv; - int argc; - int i; - - argc = scm_ilength (args); - SCM_ASSERT (argc >= 0, args, argn, subr); - argv = (char **) scm_malloc ((argc + 1) * sizeof (char *)); - for (i = 0; !SCM_NULL_OR_NIL_P (args); args = SCM_CDR (args), ++i) { - SCM arg = SCM_CAR (args); - size_t len; - char *dst; - char *src; - - SCM_ASSERT (SCM_STRINGP (arg), args, argn, subr); - len = SCM_STRING_LENGTH (arg); - src = SCM_STRING_CHARS (arg); - dst = (char *) scm_malloc (len + 1); - memcpy (dst, src, len); - dst[len] = 0; - argv[i] = dst; - } - - if (argcp) - *argcp = argc; - argv[argc] = 0; - return argv; -} - -static void -scm_free_argv (char **argv) -{ - char **av = argv; - while (*av) - free (*(av++)); - free (argv); -} - /* Dispatch to the system dependent files * * They define some static functions. These functions are called with @@ -372,6 +318,35 @@ SCM_DEFINE (scm_dynamic_call, "dynamic-call", 2, 0, 0, } #undef FUNC_NAME +/* return a newly allocated array of char pointers to each of the strings + in args, with a terminating NULL pointer. */ +/* Note: a similar function is defined in posix.c, but we don't necessarily + want to export it. */ +static char **allocate_string_pointers (SCM args, int *num_args_return) +{ + char **result; + int n_args = scm_ilength (args); + int i; + + SCM_ASSERT (n_args >= 0, args, SCM_ARGn, "allocate_string_pointers"); + result = (char **) scm_malloc ((n_args + 1) * sizeof (char *)); + result[n_args] = NULL; + for (i = 0; i < n_args; i++) + { + SCM car = SCM_CAR (args); + + if (!SCM_STRINGP (car)) + { + free (result); + scm_wrong_type_arg ("allocate_string_pointers", SCM_ARGn, car); + } + result[i] = SCM_STRING_CHARS (SCM_CAR (args)); + args = SCM_CDR (args); + } + *num_args_return = n_args; + return result; +} + SCM_DEFINE (scm_dynamic_args_call, "dynamic-args-call", 3, 0, 0, (SCM func, SCM dobj, SCM args), "Call the C function indicated by @var{func} and @var{dobj},\n" @@ -397,9 +372,12 @@ SCM_DEFINE (scm_dynamic_args_call, "dynamic-args-call", 3, 0, 0, fptr = (int (*) (int, char **)) SCM_NUM2ULONG (1, func); SCM_DEFER_INTS; - argv = scm_make_argv_from_stringlist (args, &argc, FUNC_NAME, SCM_ARG3); + argv = allocate_string_pointers (args, &argc); + /* if the procedure mutates its arguments, the original strings will be + changed -- in Guile 1.6 and earlier, this wasn't the case since a + new copy of each string was allocated. */ result = (*fptr) (argc, argv); - scm_free_argv (argv); + free (argv); SCM_ALLOW_INTS; return SCM_MAKINUM (0L + result); diff --git a/libguile/posix.c b/libguile/posix.c index 3e2334898..066e0f90b 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -827,37 +827,32 @@ SCM_DEFINE (scm_tcsetpgrp, "tcsetpgrp", 2, 0, 0, #undef FUNC_NAME #endif /* HAVE_TCSETPGRP */ -/* Create a new C argv array from a scheme list of strings. */ -/* Dirk:FIXME:: A quite similar function is implemented in dynl.c */ -/* Dirk:FIXME:: In case of assertion errors, we get memory leaks */ - -static char ** -scm_convert_exec_args (SCM args, int argn, const char *subr) +/* return a newly allocated array of char pointers to each of the strings + in args, with a terminating NULL pointer. */ +/* Note: a similar function is defined in dynl.c, but we don't necessarily + want to export it. */ +static char **allocate_string_pointers (SCM args) { - char **argv; - int argc; + char **result; + int n_args = scm_ilength (args); int i; - argc = scm_ilength (args); - SCM_ASSERT (argc >= 0, args, argn, subr); - argv = (char **) scm_malloc ((argc + 1) * sizeof (char *)); - for (i = 0; !SCM_NULLP (args); args = SCM_CDR (args), ++i) + SCM_ASSERT (n_args >= 0, args, SCM_ARGn, "allocate_string_pointers"); + result = (char **) scm_malloc ((n_args + 1) * sizeof (char *)); + result[n_args] = NULL; + for (i = 0; i < n_args; i++) { - SCM arg = SCM_CAR (args); - size_t len; - char *dst; - char *src; + SCM car = SCM_CAR (args); - SCM_ASSERT (SCM_STRINGP (arg), args, argn, subr); - len = SCM_STRING_LENGTH (arg); - src = SCM_STRING_CHARS (arg); - dst = (char *) scm_malloc (len + 1); - memcpy (dst, src, len); - dst[len] = 0; - argv[i] = dst; + if (!SCM_STRINGP (car)) + { + free (result); + scm_wrong_type_arg ("allocate_string_pointers", SCM_ARGn, car); + } + result[i] = SCM_STRING_CHARS (SCM_CAR (args)); + args = SCM_CDR (args); } - argv[i] = 0; - return argv; + return result; } SCM_DEFINE (scm_execl, "execl", 1, 0, 1, @@ -875,7 +870,7 @@ SCM_DEFINE (scm_execl, "execl", 1, 0, 1, { char **execargv; SCM_VALIDATE_STRING (1, filename); - execargv = scm_convert_exec_args (args, SCM_ARG2, FUNC_NAME); + execargv = allocate_string_pointers (args); execv (SCM_STRING_CHARS (filename), execargv); SCM_SYSERROR; /* not reached. */ @@ -895,7 +890,7 @@ SCM_DEFINE (scm_execlp, "execlp", 1, 0, 1, { char **execargv; SCM_VALIDATE_STRING (1, filename); - execargv = scm_convert_exec_args (args, SCM_ARG2, FUNC_NAME); + execargv = allocate_string_pointers (args); execvp (SCM_STRING_CHARS (filename), execargv); SCM_SYSERROR; /* not reached. */ @@ -948,7 +943,7 @@ SCM_DEFINE (scm_execle, "execle", 2, 0, 1, SCM_VALIDATE_STRING (1, filename); - execargv = scm_convert_exec_args (args, SCM_ARG1, FUNC_NAME); + execargv = allocate_string_pointers (args); exec_env = environ_list_to_c (env, SCM_ARG2, FUNC_NAME); execve (SCM_STRING_CHARS (filename), execargv, exec_env); SCM_SYSERROR;