Hi lianbo,
On Sat, Oct 11, 2025 at 4:26 PM lijiang <lijiang(a)redhat.com> wrote:
Hi, Tao
Thank you for the update.
On Mon, Sep 29, 2025 at 12:11 PM <devel-request(a)lists.crash-utility.osci.io>
wrote:
>
> Date: Mon, 29 Sep 2025 17:08:09 +1300
> From: Tao Liu <ltao(a)redhat.com>
> Subject: [Crash-utility] [PATCH v2 1/2] extensions: Search all
> possible paths
> To: devel(a)lists.crash-utility.osci.io
> Cc: jbrisson(a)linux.ibm.com
> Message-ID: <20250929040810.25718-2-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> Search all possible paths for extensions. Previously if one path, e.g.
> "/usr/lib64/crash/extensions" exists, but no extensions found within,
> crash will not search any later paths, e.g. "./extensions". This patch
> will let crash continue to search any later paths.
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> extensions.c | 63 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/extensions.c b/extensions.c
> index d23b1e3..0493f19 100644
> --- a/extensions.c
> +++ b/extensions.c
> @@ -19,7 +19,7 @@
> #include <dlfcn.h>
>
> static int in_extensions_library(char *, char *);
> -static char *get_extensions_directory(char *);
> +static char *get_extensions_directory(char *, bool *);
> static void show_all_extensions(void);
> static void show_extensions(char *);
>
> @@ -395,32 +395,33 @@ in_extensions_library(char *lib, char *buf)
> * Look for an extensions directory using the proper order.
> */
> static char *
> -get_extensions_directory(char *dirbuf)
> +get_extensions_directory(char *dirbuf, bool *end)
> {
> - char *env;
> + static int index = 0;
> + char *dirs[] = {
> + getenv("CRASH_EXTENSIONS"),
> + "/usr/lib64/crash/extensions",
> + "/usr/lib/crash/extensions",
> + "./extensions",
> + };
> + char *dir;
>
> - if ((env = getenv("CRASH_EXTENSIONS"))) {
> - if (is_directory(env)) {
> - strcpy(dirbuf, env);
> - return dirbuf;
> - }
> +retry:
> + if (index >= sizeof(dirs) / sizeof(char *)) {
> + *end = true;
> + return NULL;
> }
> -
> - if (BITS64()) {
> - sprintf(dirbuf, "/usr/lib64/crash/extensions");
> - if (is_directory(dirbuf))
> - return dirbuf;
> + *end = false;
> + dir = dirs[index++];
> + if (is_directory(dir)) {
> + if (!BITS64() && strstr(dir, "lib64")) {
> + goto retry;
> + }
> + snprintf(dirbuf, BUFSIZE - 1, "%s", dir);
> + return dir;
> + } else {
> + return NULL;
> }
> -
> - sprintf(dirbuf, "/usr/lib/crash/extensions");
> - if (is_directory(dirbuf))
> - return dirbuf;
> -
> - sprintf(dirbuf, "./extensions");
> - if (is_directory(dirbuf))
> - return dirbuf;
> -
> - return NULL;
> }
>
The above code attempts to use a goto loop, this is hard to understand and debug.
I refactored the original get_extensions_directory() as below, but I haven't tested
it. Can you try it?
Thanks for your improvement on the code, but the improvement doesn't
totally fit my attempt.
+static char* get_extensions_directory(char *dirbuf, size_t sz)
+{
+ const char *dirs[] = {
+ getenv("CRASH_EXTENSIONS"),
+ BITS64() ? "/usr/lib64/crash/extensions" :
"/usr/lib/crash/extensions",
+ "./extensions",
+ NULL
+ };
The code change looks OK to me, to add the BITS64() check within the array.
+
+ for (const char **p = dirs; *p; ++p) {
+ if (is_directory(*p)) {
+ snprintf(dirbuf, sz, "%s", *p);
+ return dirbuf;
+ }
+ }
The code change won't work, let me explain:
The job of get_extensions_directory(), is to return the possible
directories to preload_extensions().
preload_extensions() will check the directories returned by
get_extensions_directory(), to see if there is extension.so within the
dir. If there is no, then preload_extensions() need to check another
dir returned by get_extensions_directory(). In other words,
preload_extensions() will call get_extensions_directory() several
times, each time return and check one dir.
So the for-loop shouldn't within get_extensions_directory(). It should
be: call get_extensions_directory() 1st, return
getenv("CRASH_EXTENSIONS"), call get_extensions_directory() 2nd,
return "/usr/lib64/crash/extensions" etc. That's why I used a "static
int index = 0;" in get_extensions_directory().
I will send v3 for the improvement.
Thanks,
Tao Liu
+ return NULL;
+}
And also modify the is_directory(), for example:
int
-is_directory(char *file)
+is_directory(const char *file)
{
struct stat sbuf;
Anyway I'm still not sure if that can meet your needs.
Thanks
Lianbo
>
> @@ -432,14 +433,20 @@ preload_extensions(void)
> char dirbuf[BUFSIZE];
> char filename[BUFSIZE*2];
> int found;
> + bool end;
>
> - if (!get_extensions_directory(dirbuf))
> - return;
> +next_dir:
> + if (!get_extensions_directory(dirbuf, &end)) {
> + if (end)
> + return;
> + else
> + goto next_dir;
> + }
>
> dirp = opendir(dirbuf);
> if (!dirp) {
> error(INFO, "%s: %s\n", dirbuf, strerror(errno));
> - return;
> + goto next_dir;
> }
>
> pc->curcmd = pc->program_name;
> @@ -461,10 +468,12 @@ preload_extensions(void)
>
> if (found)
> fprintf(fp, "\n");
> - else
> + else {
> error(NOTE,
> "%s: no extension modules found in directory\n\n",
> dirbuf);
> + goto next_dir;
> + }
> }
>
> /*
> --
> 2.47.0