Hi lianbo,
On Sat, Oct 11, 2025 at 4:26 PM lijiang <[email protected]> wrote:
>
> Hi, Tao
> Thank you for the update.
> On Mon, Sep 29, 2025 at 12:11 PM <[email protected]>
> wrote:
>>
>> Date: Mon, 29 Sep 2025 17:08:09 +1300
>> From: Tao Liu <[email protected]>
>> Subject: [Crash-utility] [PATCH v2 1/2] extensions: Search all
>> possible paths
>> To: [email protected]
>> Cc: [email protected]
>> Message-ID: <[email protected]>
>> 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 <[email protected]>
>> ---
>> 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
--
Crash-utility mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
