I think it's a good idea for the type builtin to distinguish between
static and loadable builtins, and for debugging scripts that use
loadable builtins, it would be useful to be able to see which file got
loaded. For example in cases where BASH_LOADABLES_PATH was
unintentionally unset or set to a wrong path.
I had a go at implementing it and cobbled together the attached patch.
It extends struct builtin with a new path field that gets set to the
path that was passed to dlopen. The type builtin then includes that path
in its output if it is set.
$ ./bash -c 'enable csv; type [ csv'
[ is a shell builtin
csv is a shell builtin (/usr/local/lib/bash/csv)
$ BASH_LOADABLES_PATH=$PWD/examples/loadables ./bash -c 'enable csv; type [
csv'
[ is a shell builtin
csv is a shell builtin (/Users/geirha/bash/examples/loadables/csv)
Continuing on the debugging aspect, I think it would also be useful if
enable would warn or fail if you try to enable a loadable builtin that
was built against a different bash version. The easiest is probably to
embed the bash version inside the loadable file somehow, and compare
that when loading. Giving something like:
$ enable csv
bash: enable: /usr/local/lib/bash/csv: builtin was built for bash-5.1.16
Not sure how to go about implementing that though, or if it's even a
good approach. After all, you could potentially have multiple bash
builds of the same version with different configure options, which could
potentially leave out some function the loadable builtin expects to use.
--
Geir Hauge
diff --git a/builtins.h b/builtins.h
index 01565935..b4c71274 100644
--- a/builtins.h
+++ b/builtins.h
@@ -57,6 +57,7 @@ struct builtin {
char * const *long_doc; /* NULL terminated array of strings. */
const char *short_doc; /* Short version of documentation. */
char *handle; /* for future use */
+ char *path; /* path of loadable builtin. */
};
/* Found in builtins.c, created by builtins/mkbuiltins. */
diff --git a/builtins/enable.def b/builtins/enable.def
index 27d341a6..ae861b96 100644
--- a/builtins/enable.def
+++ b/builtins/enable.def
@@ -340,6 +340,7 @@ dyn_load_builtin (list, flags, filename)
#endif
handle = 0;
+ load_path = 0;
if (absolute_program (filename) == 0)
{
loadables_path = get_string_value ("BASH_LOADABLES_PATH");
@@ -353,7 +354,6 @@ dyn_load_builtin (list, flags, filename)
#else
handle = dlopen (load_path, RTLD_LAZY);
#endif /* !_AIX */
- free (load_path);
}
}
}
@@ -377,6 +377,8 @@ dyn_load_builtin (list, flags, filename)
if (name != filename)
free (name);
}
+ if (load_path)
+ free (load_path);
return (EX_NOTFOUND);
}
@@ -434,6 +436,7 @@ dyn_load_builtin (list, flags, filename)
if (flags & SPECIAL)
b->flags |= SPECIAL_BUILTIN;
b->handle = handle;
+ b->path = load_path ? savestring(load_path) : NULL;
if (old_builtin)
{
@@ -444,6 +447,9 @@ dyn_load_builtin (list, flags, filename)
new_builtins[new++] = b;
}
+ if (load_path)
+ free (load_path);
+
if (replaced == 0 && new == 0)
{
free (new_builtins);
diff --git a/builtins/type.def b/builtins/type.def
index a8e47c0a..fc7099db 100644
--- a/builtins/type.def
+++ b/builtins/type.def
@@ -52,6 +52,7 @@ $END
#include <config.h>
+#include "../builtins.h"
#include "../bashtypes.h"
#include "posixstat.h"
@@ -289,6 +290,7 @@ describe_command (command, dflags)
}
/* Command is a builtin? */
+ struct builtin *b = builtin_address_internal(command, 0);
if (((dflags & CDESC_FORCE_PATH) == 0) && find_shell_builtin (command))
{
if (dflags & CDESC_TYPE)
@@ -297,6 +299,8 @@ describe_command (command, dflags)
{
if (posixly_correct && find_special_builtin (command) != 0)
printf (_("%s is a special shell builtin\n"), command);
+ else if (b->path)
+ printf (_("%s is a shell builtin (%s)\n"), command, b->path);
else
printf (_("%s is a shell builtin\n"), command);
}
