diff options
author | Peter Seebach <peter.seebach@windriver.com> | 2015-08-19 16:11:51 -0500 |
---|---|---|
committer | Peter Seebach <peter.seebach@windriver.com> | 2015-08-20 17:17:56 -0500 |
commit | 79cd74c8643eeaef52e9f78135f60a9652bcf65c (patch) | |
tree | 4bb95205e1dd7d66cc16531e58216a7d0f2ef7ca | |
parent | 4377856c427d40d4d518263ff0b6effa00fd3636 (diff) |
Drop the allocation in pseudo_fix_path/pseudo_root_path/etc.
Instead of allocating (and then freeing) these paths all the time,
use a rotating selection of buffers of fixed but probably large enough
size (the same size that would have been the maximum anyway in
general). With the exception of fts_open, there's no likely way to
end up needing more than two or three such paths at a time. fts_open
dups the paths since it could have a large number and need them for
a while. This dramatically reduces (in principle) the amount of allocation
and especially reallocation going on.
Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
-rw-r--r-- | ChangeLog.txt | 3 | ||||
-rwxr-xr-x | makewrappers | 18 | ||||
-rw-r--r-- | ports/common/guts/execv.c | 1 | ||||
-rw-r--r-- | ports/common/guts/execve.c | 1 | ||||
-rw-r--r-- | ports/common/guts/execvp.c | 1 | ||||
-rw-r--r-- | ports/linux/guts/mkstemp64.c | 1 | ||||
-rw-r--r-- | ports/unix/guts/fts_open.c | 2 | ||||
-rw-r--r-- | ports/unix/guts/linkat.c | 6 | ||||
-rw-r--r-- | ports/unix/guts/mkdtemp.c | 1 | ||||
-rw-r--r-- | ports/unix/guts/mkstemp.c | 1 | ||||
-rw-r--r-- | ports/unix/guts/mktemp.c | 1 | ||||
-rw-r--r-- | ports/unix/guts/realpath.c | 2 | ||||
-rw-r--r-- | pseudo.c | 8 | ||||
-rw-r--r-- | pseudo_client.c | 7 | ||||
-rw-r--r-- | pseudo_util.c | 71 | ||||
-rw-r--r-- | templates/wrapfuncs.c | 3 |
16 files changed, 41 insertions, 86 deletions
diff --git a/ChangeLog.txt b/ChangeLog.txt index ff27760..1ac657c 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,6 @@ +2015-08-19: + * (seebs) Reduce alloc/free cycles in path computations. + 2015-08-17: * (seebs) profiling improvements diff --git a/makewrappers b/makewrappers index efe6180..15b4274 100755 --- a/makewrappers +++ b/makewrappers @@ -340,27 +340,17 @@ class Function: """present argument list for a function call""" return self.args.call() - def alloc_paths(self): + def fix_paths(self): """create/allocate canonical paths""" - alloc_paths = [] + fix_paths = [] for path in self.paths_to_munge: prefix = path[:-4] if prefix not in self.specific_dirfds: prefix = '' - alloc_paths.append( + fix_paths.append( "%s = pseudo_root_path(__func__, __LINE__, %s%s, %s, %s);" % (path, prefix, self.dirfd, path, self.flags)) - return "\n\t\t".join(alloc_paths) - - def free_paths(self): - """free any allocated paths""" - free_paths = [] - # the cast is here because free's argument isn't const qualified, but - # the original path may have been -- but we only GET here if the path - # has been overwritten. - for path in self.paths_to_munge: - free_paths.append("free((void *) %s);" % path) - return "\n\t\t".join(free_paths) + return "\n\t\t".join(fix_paths) def real_predecl(self): if self.real_func: diff --git a/ports/common/guts/execv.c b/ports/common/guts/execv.c index 3e1f820..d5dc3b0 100644 --- a/ports/common/guts/execv.c +++ b/ports/common/guts/execv.c @@ -15,7 +15,6 @@ if (antimagic == 0) { char *path_guess = pseudo_exec_path(file, 0); pseudo_client_op(OP_EXEC, PSA_EXEC, -1, -1, path_guess, 0); - free(path_guess); } pseudo_setupenv(); diff --git a/ports/common/guts/execve.c b/ports/common/guts/execve.c index ff6a44e..22d07a2 100644 --- a/ports/common/guts/execve.c +++ b/ports/common/guts/execve.c @@ -16,7 +16,6 @@ if (antimagic == 0) { char *path_guess = pseudo_exec_path(file, 0); pseudo_client_op(OP_EXEC, PSA_EXEC, -1, -1, path_guess, 0); - free(path_guess); } new_environ = pseudo_setupenvp(envp); diff --git a/ports/common/guts/execvp.c b/ports/common/guts/execvp.c index 04253c3..b30705d 100644 --- a/ports/common/guts/execvp.c +++ b/ports/common/guts/execvp.c @@ -16,7 +16,6 @@ if (antimagic == 0) { char *path_guess = pseudo_exec_path(file, 1); pseudo_client_op(OP_EXEC, PSA_EXEC, -1, -1, path_guess, 0); - free(path_guess); } pseudo_setupenv(); diff --git a/ports/linux/guts/mkstemp64.c b/ports/linux/guts/mkstemp64.c index 6497f2e..cbeda0e 100644 --- a/ports/linux/guts/mkstemp64.c +++ b/ports/linux/guts/mkstemp64.c @@ -41,7 +41,6 @@ } /* mkstemp only changes the XXXXXX at the end. */ memcpy(template + len - 6, tmp_template + strlen(tmp_template) - 6, 6); - free(tmp_template); /* return rc; * } */ diff --git a/ports/unix/guts/fts_open.c b/ports/unix/guts/fts_open.c index 8b3ce19..964314e 100644 --- a/ports/unix/guts/fts_open.c +++ b/ports/unix/guts/fts_open.c @@ -29,6 +29,8 @@ rpath_argv[i] = PSEUDO_ROOT_PATH(AT_FDCWD, path_argv[i], AT_SYMLINK_NOFOLLOW); if (!rpath_argv[i]) errored = 1; + else + rpath_argv[i] = strdup(rpath_argv[i]); } if (errored) { diff --git a/ports/unix/guts/linkat.c b/ports/unix/guts/linkat.c index 9ca7c47..fe29b39 100644 --- a/ports/unix/guts/linkat.c +++ b/ports/unix/guts/linkat.c @@ -37,8 +37,6 @@ rc = real_link(oldpath, newpath); save_errno = errno; if (rc == -1) { - free(oldpath); - free(newpath); errno = save_errno; return rc; } @@ -53,8 +51,6 @@ if (rc2 == -1) { pseudo_diag("Fatal: Tried to stat '%s' after linking it, but failed: %s.\n", oldpath, strerror(errno)); - free(oldpath); - free(newpath); errno = ENOENT; return rc; } @@ -67,8 +63,6 @@ */ pseudo_client_op(OP_LINK, 0, -1, -1, newpath, &buf); - free(oldpath); - free(newpath); errno = save_errno; /* return rc; diff --git a/ports/unix/guts/mkdtemp.c b/ports/unix/guts/mkdtemp.c index 5ef647b..5337661 100644 --- a/ports/unix/guts/mkdtemp.c +++ b/ports/unix/guts/mkdtemp.c @@ -40,7 +40,6 @@ /* mkdtemp only changes the XXXXXX at the end. */ memcpy(template + len - 6, tmp_template + strlen(tmp_template) - 6, 6); rc = template; - free(tmp_template); /* return rc; * } */ diff --git a/ports/unix/guts/mkstemp.c b/ports/unix/guts/mkstemp.c index 1f055fd..315bec8 100644 --- a/ports/unix/guts/mkstemp.c +++ b/ports/unix/guts/mkstemp.c @@ -41,7 +41,6 @@ } /* mkstemp only changes the XXXXXX at the end. */ memcpy(template + len - 6, tmp_template + strlen(tmp_template) - 6, 6); - free(tmp_template); /* return rc; * } */ diff --git a/ports/unix/guts/mktemp.c b/ports/unix/guts/mktemp.c index 7cf594a..a39d1b7 100644 --- a/ports/unix/guts/mktemp.c +++ b/ports/unix/guts/mktemp.c @@ -29,7 +29,6 @@ */ memcpy(template + len - 6, tmp_template + strlen(tmp_template) - 6, 6); rc = template; - free(tmp_template); /* return rc; * } diff --git a/ports/unix/guts/realpath.c b/ports/unix/guts/realpath.c index 8059f5e..48b2b34 100644 --- a/ports/unix/guts/realpath.c +++ b/ports/unix/guts/realpath.c @@ -13,13 +13,11 @@ return NULL; } if ((len = strlen(rname)) >= pseudo_sys_path_max()) { - free(rname); errno = ENAMETOOLONG; return NULL; } if (resolved_name) { memcpy(resolved_name, rname, len + 1); - free(rname); rc = resolved_name; } else { rc = rname; @@ -180,7 +180,7 @@ main(int argc, char *argv[]) { pseudo_diag("Can't resolve path '%s'\n", optarg); usage(EXIT_FAILURE); } - opt_i = s; + opt_i = strdup(s); break; case 'l': /* log */ optptr += snprintf(optptr, pseudo_path_max() - (optptr - opts), @@ -193,7 +193,7 @@ main(int argc, char *argv[]) { pseudo_diag("Can't resolve move-from path '%s'\n", optarg); usage(EXIT_FAILURE); } - opt_m = s; + opt_m = strdup(s); break; case 'M': /* move to... (see also 'm') */ s = PSEUDO_ROOT_PATH(AT_FDCWD, optarg, 0); @@ -201,7 +201,7 @@ main(int argc, char *argv[]) { pseudo_diag("Can't resolve move-to path '%s'\n", optarg); usage(EXIT_FAILURE); } - opt_M = s; + opt_M = strdup(s); break; case 'p': /* passwd file path */ s = PSEUDO_ROOT_PATH(AT_FDCWD, optarg, AT_SYMLINK_NOFOLLOW); @@ -228,7 +228,7 @@ main(int argc, char *argv[]) { } pseudo_set_value("PSEUDO_CHROOT", s); if (o == 'r') - opt_r = s; + opt_r = strdup(s); break; case 'S': /* stop */ opt_S = 1; diff --git a/pseudo_client.c b/pseudo_client.c index c4dd43d..7f30467 100644 --- a/pseudo_client.c +++ b/pseudo_client.c @@ -1226,7 +1226,7 @@ base_path(int dirfd, const char *path, int leave_last) { if (!path) return NULL; if (!*path) - return strdup(""); + return ""; if (path[0] != '/') { if (dirfd != -1 && dirfd != AT_FDCWD) { @@ -1746,18 +1746,15 @@ pseudo_exec_path(const char *filename, int search_path) { pseudo_diag("couldn't allocate intermediate path.\n"); candidate = NULL; } - free(dir); } if (candidate && !stat(candidate, &buf) && !S_ISDIR(buf.st_mode) && (buf.st_mode & 0111)) { pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "exec_path: %s => %s\n", filename, candidate); pseudo_magic(); return candidate; - } else { - free(candidate); } } /* blind guess being as good as anything */ pseudo_magic(); - return strdup(filename); + return filename; } diff --git a/pseudo_util.c b/pseudo_util.c index ccbda60..865222a 100644 --- a/pseudo_util.c +++ b/pseudo_util.c @@ -222,8 +222,8 @@ int pseudo_util_debug_fd = 2; static int debugged_newline = 1; static char pid_text[32]; static size_t pid_len; -static int pseudo_append_element(char **pnewpath, char **proot, size_t *pallocated, char **pcurrent, const char *element, size_t elen, int leave_this); -static int pseudo_append_elements(char **newpath, char **root, size_t *allocated, char **current, const char *elements, size_t elen, int leave_last); +static int pseudo_append_element(char *newpath, char *root, size_t allocated, char **pcurrent, const char *element, size_t elen, int leave_this); +static int pseudo_append_elements(char *newpath, char *root, size_t allocated, char **current, const char *elements, size_t elen, int leave_last); extern char **environ; static ssize_t pseudo_max_pathlen = -1; static ssize_t pseudo_sys_max_pathlen = -1; @@ -465,22 +465,18 @@ pseudo_new_pid() { * the symlink, appending each element in turn the same way. */ static int -pseudo_append_element(char **pnewpath, char **proot, size_t *pallocated, char **pcurrent, const char *element, size_t elen, int leave_this) { +pseudo_append_element(char *newpath, char *root, size_t allocated, char **pcurrent, const char *element, size_t elen, int leave_this) { static int link_recursion = 0; - size_t curlen, allocated; - char *newpath, *current, *root; + size_t curlen; + char *current; struct stat buf; - if (!pnewpath || !*pnewpath || + if (!newpath || !pcurrent || !*pcurrent || - !proot || !*proot || - !pallocated || !element) { + !root || !element) { pseudo_diag("pseudo_append_element: invalid args.\n"); return -1; } - newpath = *pnewpath; - allocated = *pallocated; current = *pcurrent; - root = *proot; /* sanity-check: ignore // or /./ */ if (elen == 0 || (elen == 1 && *element == '.')) { return 1; @@ -505,23 +501,8 @@ pseudo_append_element(char **pnewpath, char **proot, size_t *pallocated, char ** /* current length, plus / <element> / \0 */ /* => curlen + elen + 3 */ if (curlen + elen + 3 > allocated) { - char *bigger; - size_t big = round_up(allocated + elen, 256); - bigger = malloc(big); - if (!bigger) { - pseudo_diag("pseudo_append_element: couldn't allocate space (wanted %lu bytes).\n", (unsigned long) big); - return -1; - } - memcpy(bigger, newpath, curlen); - current = bigger + curlen; - root = bigger + (root - newpath); - free(newpath); - newpath = bigger; - allocated = big; - *pnewpath = newpath; - *pcurrent = current; - *proot = root; - *pallocated = allocated; + pseudo_diag("pseudo_append_element: path too long (wanted %lu bytes).\n", (unsigned long) curlen + elen + 3); + return -1; } memcpy(current, element, elen); current += elen; @@ -560,7 +541,7 @@ pseudo_append_element(char **pnewpath, char **proot, size_t *pallocated, char ** /* append all the elements in series */ *pcurrent = current; ++link_recursion; - retval = pseudo_append_elements(pnewpath, proot, pallocated, pcurrent, linkbuf, linklen, 0); + retval = pseudo_append_elements(newpath, root, allocated, pcurrent, linkbuf, linklen, 0); --link_recursion; return retval; } @@ -573,11 +554,10 @@ pseudo_append_element(char **pnewpath, char **proot, size_t *pallocated, char ** } static int -pseudo_append_elements(char **newpath, char **root, size_t *allocated, char **current, const char *element, size_t elen, int leave_last) { +pseudo_append_elements(char *newpath, char *root, size_t allocated, char **current, const char *element, size_t elen, int leave_last) { int retval = 1; const char * start = element; - if (!newpath || !*newpath || - !root || !*root || + if (!newpath || !root || !current || !*current || !element) { pseudo_diag("pseudo_append_elements: invalid arguments."); @@ -614,6 +594,11 @@ pseudo_append_elements(char **newpath, char **root, size_t *allocated, char **cu return retval; } +/* don't do so many allocations */ +#define PATHBUFS 16 +static char *pathbufs[PATHBUFS] = { 0 }; +static int pathbuf = 0; + /* Canonicalize path. "base", if present, is an already-canonicalized * path of baselen characters, presumed not to end in a /. path is * the new path to be canonicalized. The tricky part is that path may @@ -633,29 +618,26 @@ pseudo_fix_path(const char *base, const char *path, size_t rootlen, size_t basel pseudo_diag("can't fix empty path.\n"); return 0; } + newpathlen = pseudo_path_max(); + if (!pathbufs[pathbuf]) { + pathbufs[pathbuf] = malloc(newpathlen); + } + newpath = pathbufs[pathbuf]; + pathbuf = (pathbuf + 1) % PATHBUFS; pathlen = strlen(path); /* a trailing slash has special meaning */ if (pathlen > 0 && path[pathlen - 1] == '/') { trailing_slash = 1; } - newpathlen = pathlen; - /* If the path starts with /, we don't care about base, UNLESS - * rootlen is non-zero, in which case we're doing a chroot thing - * and will actually need to append some components. - */ - if (baselen && (path[0] != '/' || rootlen)) { - newpathlen += baselen + 2; - } /* allow a bit of slush. overallocating a bit won't * hurt. rounding to 256's in the hopes that it makes life * easier for the library. */ - newpathlen = round_up(newpathlen, 256); - newpath = malloc(newpathlen); if (!newpath) { pseudo_diag("allocation failed seeking memory for path (%s).\n", path); return 0; } + newpath[0] = '\0'; current = newpath; if (baselen && (path[0] != '/' || rootlen)) { memcpy(current, base, baselen); @@ -673,7 +655,7 @@ pseudo_fix_path(const char *base, const char *path, size_t rootlen, size_t basel * newpathlen is the total allocated length of newpath * (current - newpath) is the used length of newpath */ - if (pseudo_append_elements(&newpath, &effective_root, &newpathlen, ¤t, path, pathlen, leave_last) != -1) { + if (pseudo_append_elements(newpath, effective_root, newpathlen, ¤t, path, pathlen, leave_last) != -1) { --current; if (*current == '/' && current > effective_root && !trailing_slash) { *current = '\0'; @@ -687,7 +669,6 @@ pseudo_fix_path(const char *base, const char *path, size_t rootlen, size_t basel } return newpath; } else { - free(newpath); return 0; } } @@ -1070,10 +1051,8 @@ pseudo_get_prefix(char *pathname) { if ((int) strlen(tmp_path) >= pseudo_path_max()) { pseudo_diag("Can't expand path '%s' -- expansion exceeds %d.\n", mypath, (int) pseudo_path_max()); - free(tmp_path); } else { s = mypath + snprintf(mypath, pseudo_path_max(), "%s", tmp_path); - free(tmp_path); } while (s > (mypath + 1) && *s != '/') diff --git a/templates/wrapfuncs.c b/templates/wrapfuncs.c index 5a436e9..2d13031 100644 --- a/templates/wrapfuncs.c +++ b/templates/wrapfuncs.c @@ -59,11 +59,10 @@ ${maybe_async_skip} pseudo_debug(PDBGF_SYSCALL, "${name} calling real syscall.\n"); ${rc_assign} (*real_${name})(${call_args}); } else { - ${alloc_paths} + ${fix_paths} /* exec*() use this to restore the sig mask */ pseudo_saved_sigmask = saved; ${rc_assign} wrap_$name(${call_args}); - ${free_paths} } ${variadic_end} save_errno = errno; |