Fix the defpath memory leak ASan keeps complaining about

[JohnoKing]

Currently, running ksh under ASan without the ASAN_OPTIONS variable set to detect_leaks=0 usually ends with ASan complaining about a memory leak in defpathinit() (this leak doesn't grow in a loop, so no regression test was added to leaks.sh). Reproducer:

$ ENV=/dev/null arch/*/bin/ksh
$ cp -?
cp: invalid option -- '?'
Try 'cp --help' for more information.
$ exit

=================================================================
==225132==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 85 byte(s) in 1 object(s) allocated from:
    #0 0x7f6dab42d459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x5647b77fe144 in sh_calloc /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/init.c:265
    #2 0x5647b788fea9 in path_addcomp /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1567
    #3 0x5647b78911ed in path_addpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1705
    #4 0x5647b7888e82 in defpathinit /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:442
    #5 0x5647b78869f3 in ondefpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:67
    #6 0x5647b7888225 in checkdup /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:365
    #7 0x5647b7888714 in path_nextcomp /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:403
    #8 0x5647b788b2c0 in path_absolute /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:785
    #9 0x5647b788ad8d in path_search /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:727
    #10 0x5647b78b5549 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1204
    #11 0x5647b77b8032 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:591
    #12 0x5647b77b5c26 in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:352
    #13 0x5647b77b3ffd in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:45
    #14 0x7f6dab0ae6cd in __libc_start_call_main (/usr/lib/libc.so.6+0x2d6cd)

Indirect leak of 89 byte(s) in 1 object(s) allocated from:
    #0 0x7f6dab42d459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x5647b77fe144 in sh_calloc /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/init.c:265
    #2 0x5647b788fea9 in path_addcomp /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1567
    #3 0x5647b78911ed in path_addpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1705
    #4 0x5647b7888e82 in defpathinit /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:442
    #5 0x5647b78869f3 in ondefpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:67
    #6 0x5647b7888225 in checkdup /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:365
    #7 0x5647b7888714 in path_nextcomp /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:403
    #8 0x5647b788b2c0 in path_absolute /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:785
    #9 0x5647b788ad8d in path_search /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:727
    #10 0x5647b78b5549 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1204
    #11 0x5647b77b8032 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:591
    #12 0x5647b77b5c26 in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:352
    #13 0x5647b77b3ffd in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:45
    #14 0x7f6dab0ae6cd in __libc_start_call_main (/usr/lib/libc.so.6+0x2d6cd)

SUMMARY: AddressSanitizer: 174 byte(s) leaked in 2 allocation(s).

src/cmd/ksh93/sh/path.c:
- Move the code for allocating defpath from defpathinit() into its own dedicated function called std_path(). This function is called by defpathinit() and ondefpath() to obtain the current string stored in the defpath variable. This bugfix is adapted from a fork of ksh2020: l0stman/ksh@db5c83a

[McDutchie]

This makes no sense. Please explain how this is a memory leak at all.

The string buffer returned by the astconf call should be duplicated, otherwise it will be overwritten. And it's done once at most. There is no point in ever freeing it as the default path string should be kept until the process exits.

As far as I can tell, the new code does exactly the same thing as the old, just in a less straightforward way.

[JohnoKing]

The previous code was leaking memory because defpathinit() returns a pointer from path_addpath(), which has memory allocated to it in path_addcomp(). This is the code ASan traced as having allocated memory:
In defpathinit():

ksh/src/cmd/ksh93/sh/path.c

Line 442 in daf483d

return(path_addpath((Pathcomp_t*)0,(defpath),PATH_PATH));

In path_addpath():

ksh/src/cmd/ksh93/sh/path.c

Line 1705 in daf483d

first = path_addcomp(first,old,cp,type);

ksh/src/cmd/ksh93/sh/path.c

Line 1729 in daf483d

return(first);

In path_addcomp():

ksh/src/cmd/ksh93/sh/path.c

Line 1567 in daf483d

pp = sh_newof((Pathcomp_t*)0,Pathcomp_t,1,len+1);

The ondefpath() function doesn't save a reference to the pointer returned by defpathinit(), which causes the memory leak:

ksh/src/cmd/ksh93/sh/path.c

Lines 66 to 67 in daf483d

	if(!defpath)
	defpathinit();

The changes in this pull request avoid this problem by setting the defpath variable without also calling path_addpath().

[McDutchie]

Thanks for the explanation. So it wasn't complaining about the sh_strdup() call in defpathinit() at all, that makes sense now. And I should have seen that from the trace you posted.