Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: PATH: autoload with explicit path



On Wed, 11 Jan 2017 20:51:22 +0000
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> With a bit of work this can be expanded to paths for already loaded
> functions, and possibly elsewhere though I think the pickings are
> a lot smaller.

This does the former of that --- it was less work than I expected.

The upshot of this is that with the new absolute path scheme, storing
the filename at the point of loading the function is a no-op since we've
already got the components and keep them.  With a traditional
fpath-style load, we use the new directory cache at the point where we
load the file from the path, so this now also avoids an extra directory
allocation (and, unlike the similar mechanism used for the command hash,
avoids the brittle association with the original path variable).

There is a heap allocation when we enter the function to put the name on
the function stack (see getshfuncfile()), but there always was --- the
only difference is a little extra calculation for the three components.
So I think the penalty is trivial to non-existent.

The only significant extra requirement is use of the heap to get the
filename in "whence -v" --- that's a bit of laziness, we could just print
out the components using the same logic as getshfuncfile().  Might fix
that later.

Note that the PM_LOADDIR flag still represents exactly what it did,
it's just that the period of possible validity now lasts throughout the
function [nearly said product] life cycle.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 7a5d2bd..6c82643 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5124,12 +5124,12 @@ execautofn_basic(Estate state, UNUSED(int do_exec))
      * defined yet.
      */
     if (funcstack && !funcstack->filename)
-	funcstack->filename = dupstring(shf->filename);
+	funcstack->filename = getshfuncfile(shf);
 
     oldscriptname = scriptname;
     oldscriptfilename = scriptfilename;
     scriptname = dupstring(shf->node.nam);
-    scriptfilename = dupstring(shf->filename);
+    scriptfilename = getshfuncfile(shf);
     execode(shf->funcdef, 1, 0, "loadautofunc");
     scriptname = oldscriptname;
     scriptfilename = oldscriptfilename;
@@ -5150,13 +5150,47 @@ execautofn(Estate state, UNUSED(int do_exec))
     return execautofn_basic(state, 0);
 }
 
+/*
+ * Helper function to install the source file name of a shell function
+ * just autoloaded.
+ *
+ * We attempt to do this efficiently as the typical case is the
+ * directory part is a well-known directory, which is cached, and
+ * the non-directory part is the same as the node name.
+ */
+
+/**/
+static void
+loadautofnsetfile(Shfunc shf, char *fdir)
+{
+    /*
+     * If shf->filename is already the load directory ---
+     * keep it as we can still use it to get the load file.
+     * This makes autoload with an absolute path particularly efficient.
+     */
+    if (!(shf->node.flags & PM_LOADDIR) ||
+	strcmp(shf->filename, fdir) != 0) {
+	/* Old directory name not useful... */
+	dircache_set(&shf->filename, NULL);
+	if (fdir) {
+	    /* ...can still cache directory */
+	    shf->node.flags |= PM_LOADDIR;
+	    dircache_set(&shf->filename, fdir);
+	} else {
+	    /* ...no separate directory part to cache, for some reason. */
+	    shf->node.flags &= ~PM_LOADDIR;
+	    shf->filename = ztrdup(shf->node.nam);
+	}
+    }
+}
+
 /**/
 Shfunc
 loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 {
     int noalias = noaliases, ksh = 1;
     Eprog prog;
-    char *fname;
+    char *fdir;			/* Directory path where func found */
 
     pushheap();
 
@@ -5167,13 +5201,13 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	char *spec_path[2];
 	spec_path[0] = dupstring(shf->filename);
 	spec_path[1] = NULL;
-	prog = getfpfunc(shf->node.nam, &ksh, &fname, spec_path, 0);
+	prog = getfpfunc(shf->node.nam, &ksh, &fdir, spec_path, 0);
 	if (prog == &dummy_eprog &&
 	    (current_fpath || (shf->node.flags & PM_CUR_FPATH)))
-	    prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0);
+	    prog = getfpfunc(shf->node.nam, &ksh, &fdir, NULL, 0);
     }
     else
-	prog = getfpfunc(shf->node.nam, &ksh, &fname, NULL, 0);
+	prog = getfpfunc(shf->node.nam, &ksh, &fdir, NULL, 0);
     noaliases = noalias;
 
     if (ksh == 1) {
@@ -5192,7 +5226,6 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	return NULL;
     }
     if (!prog) {
-	zsfree(fname);
 	popheap();
 	return NULL;
     }
@@ -5205,10 +5238,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 		shf->funcdef = prog;
 	    else
 		shf->funcdef = dupeprog(prog, 0);
-	    shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR);
-	    dircache_set(&shf->filename, NULL);
-	    /* Full filename, don't use dircache */
-	    shf->filename = fname;
+	    shf->node.flags &= ~PM_UNDEFINED;
+	    loadautofnsetfile(shf, fdir);
 	} else {
 	    VARARR(char, n, strlen(shf->node.nam) + 1);
 	    strcpy(n, shf->node.nam);
@@ -5220,7 +5251,6 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 		zwarn("%s: function not defined by file", n);
 		locallevel++;
 		popheap();
-		zsfree(fname);
 		return NULL;
 	    }
 	}
@@ -5230,10 +5260,8 @@ loadautofn(Shfunc shf, int fksh, int autol, int current_fpath)
 	    shf->funcdef = stripkshdef(prog, shf->node.nam);
 	else
 	    shf->funcdef = dupeprog(stripkshdef(prog, shf->node.nam), 0);
-	shf->node.flags &= ~(PM_UNDEFINED|PM_LOADDIR);
-	dircache_set(&shf->filename, NULL);
-	/* Full filename, don't use dircache */
-	shf->filename = fname;
+	shf->node.flags &= ~PM_UNDEFINED;
+	loadautofnsetfile(shf, fdir);
     }
     popheap();
 
@@ -5453,7 +5481,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcstack = &fstack;
 
 	fstack.flineno = shfunc->lineno;
-	fstack.filename = dupstring(shfunc->filename);
+	fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;
 	if (prog->flags & EF_RUN) {
@@ -5623,16 +5651,17 @@ runshfunc(Eprog prog, FuncWrap wrap, char *name)
  * Search fpath for an undefined function.  Finds the file, and returns the
  * list of its contents.
  *
- * If test is 0, load the function; *fname is set to zalloc'ed location.
+ * If test is 0, load the function.
  *
  * If test_only is 1, don't load function, just test for it:
- * - Non-null return means function was found
- * - *fname points to path at which found (not duplicated)
+ * Non-null return means function was found
+ *
+ * *fdir points to path at which found (as passed in, not duplicated)
  */
 
 /**/
 Eprog
-getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
+getfpfunc(char *s, int *ksh, char **fdir, char **alt_path, int test_only)
 {
     char **pp, buf[PATH_MAX+1];
     off_t len;
@@ -5650,8 +5679,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
 	else
 	    strcpy(buf, s);
 	if ((r = try_dump_file(*pp, s, buf, ksh, test_only))) {
-	    if (fname)
-		*fname = test_only ? *pp : ztrdup(buf);
+	    if (fdir)
+		*fdir = *pp;
 	    return r;
 	}
 	unmetafy(buf, NULL);
@@ -5661,7 +5690,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
 		(len = lseek(fd, 0, 2)) != -1) {
 		if (test_only) {
 		    close(fd);
-		    *fname = *pp;
+		    if (fdir)
+			*fdir = *pp;
 		    return &dummy_eprog;
 		}
 		d = (char *) zalloc(len + 1);
@@ -5677,8 +5707,8 @@ getfpfunc(char *s, int *ksh, char **fname, char **alt_path, int test_only)
 		    r = parse_string(d, 1);
 		    scriptname = oldscriptname;
 
-		    if (fname)
-			*fname = ztrdup(buf);
+		    if (fdir)
+			*fdir = *pp;
 
 		    zfree(d, len + 1);
 
diff --git a/Src/hashtable.c b/Src/hashtable.c
index a3a38f7..0fc87f3 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -926,10 +926,12 @@ printshfuncnode(HashNode hn, int printflags)
 	       (f->node.flags & PM_UNDEFINED) ?
 	       " is an autoload shell function" :
 	       " is a shell function");
-	if (f->filename && (printflags & PRINT_WHENCE_VERBOSE) &&
-	    strcmp(f->filename, f->node.nam) != 0) {
-	    printf(" from ");
-	    quotedzputs(f->filename, stdout);
+	if (printflags & PRINT_WHENCE_VERBOSE) {
+	    char *fname = getshfuncfile(f);
+	    if (fname && strcmp(fname, f->node.nam) != 0) {
+		printf(" from ");
+		quotedzputs(fname, stdout);
+	    }
 	}
 	putchar('\n');
 	return;
@@ -959,7 +961,7 @@ printshfuncnode(HashNode hn, int printflags)
 	    zputs("builtin autoload -X", stdout);
 	    for (fl=0;fopt[fl];fl++)
 		if (f->node.flags & flgs[fl]) putchar(fopt[fl]);
-	    if (f->filename) {
+	    if (f->filename && (f->node.flags & PM_LOADDIR)) {
 		putchar(' ');
 		zputs(f->filename, stdout);
 	    }
@@ -1041,6 +1043,24 @@ printshfuncexpand(HashNode hn, int printflags, int expand)
     text_expand_tabs = save_expand;
 }
 
+/*
+ * Get a heap-duplicated name of the shell function, for
+ * use in tracing.
+ */
+
+/**/
+mod_export char *
+getshfuncfile(Shfunc shf)
+{
+    if (shf->node.flags & PM_LOADDIR) {
+	return zhtricat(shf->filename, "/", shf->node.nam);
+    } else if (shf->filename) {
+	return dupstring(shf->filename);
+    } else {
+	return NULL;
+    }
+}
+
 /**************************************/
 /* Reserved Word Hash Table Functions */
 /**************************************/
diff --git a/Src/signals.c b/Src/signals.c
index 2de5743..a717677 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -811,8 +811,11 @@ dosavetrap(int sig, int level)
 	    newshf->node.nam = ztrdup(shf->node.nam);
 	    newshf->node.flags = shf->node.flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
-	    /* Assume this is a real filename, don't use dircache */
-	    newshf->filename = ztrdup(shf->filename);
+	    if (shf->node.flags & PM_LOADDIR) {
+		dircache_set(&newshf->filename, shf->filename);
+	    } else {
+		newshf->filename = ztrdup(shf->filename);
+	    }
 	    if (shf->sticky) {
 		newshf->sticky = sticky_emulation_dup(shf->sticky, 0);
 	    } else



Messages sorted by: Reverse Date, Date, Thread, Author