[syslinux] PATH directive searches in reverse order with wrong separator

[Phil Pokorny]

I've been trying to get syslinux.efi working in my environment again...

Found what look like a bunch of little bugs that are very frustrating...

First, the documentation on the Wiki says that as of 5.11, the list
separator is space, not colon.  But I can find no evidence that 5.11
was ever officially released or that a commit to git was made to make
this change.  6.00 and following still use colon as the separator in
the parse routine.  This should fix that.

diff --git a/com32/elflink/ldlinux/readconfig.c
b/com32/elflink/ldlinux/readconfig.c
--- a/com32/elflink/ldlinux/readconfig.c
+++ b/com32/elflink/ldlinux/readconfig.c
@@ -786,7 +786,7 @@ static int parse_path(char *p)
        char *c = p;

        /* Find the next directory */
-       while (*c && *c != ':')
+       while (*c && !my_isspace(*c))
            c++;

        str = refstrndup(p, c - p);

Speaking of which, why does the code in elflink/ldlinux use
"my_isspace" while the rest of SYSLINUX uses isspace()?  And why is
there not_whitespace() in core/include/fs.h?

Next, path_add() adds new entries at the beginning of the list not the
end.  This causes directories to be search last first not first to
last.  This should fix that.

diff --git a/core/path.c b/core/path.c
--- a/core/path.c
+++ b/core/path.c
@@ -32,7 +32,7 @@ __export struct path_entry *path_add(const char *str)
     if (!entry->str)
        goto bail;

-    list_add(&entry->list, &PATH);
+    list_add_tail(&entry->list, &PATH);

     return entry;

But that will cause the code that attempts to put the CurrentDirName
at the beginning of the list to break because it will end up at the
end.  Probably better to move the check for current working directory
location for a file to findpath() like this:

diff --git a/com32/lib/sys/module/common.c b/com32/lib/sys/module/common.c
--- a/com32/lib/sys/module/common.c
+++ b/com32/lib/sys/module/common.c
@@ -67,6 +67,14 @@ FILE *findpath(char *name)
        if (f)
                return f;

+       /* Look in CurrentDirName */
+       /* CurrentDirName ends with a / (see efi_setcwd) */
+       snprintf(path, sizeof(path), "%s%s", CurrentDirName, name);
+       dprintf("findpath: trying \"%s\"\n", path);
+       f = fopen(path, "rb"); /* for full path */
+       if (f)
+               return f;
+
        list_for_each_entry(entry, &PATH, list) {
                bool slash = false;


but I'm worried that the code in the com32/lib/ tree expects to use
chdir,(), get_cwd() and friends to manipulate the current working
directory. (see core/fs/lib/searchconfig.c)  But that leads into a
maze of twisty passages of virtual functions, default implementations
and other stuff I haven't figured out yet.  Also, what's the
difference between CurrentDirName[] and config_cwd[] and
this_fs->cwd_name?

While trying to figure all that out, I found this in
generic_mangle_name() which looks wrong to me.  When collapsing repeat
slashes (should we also be stripping repeat backslashes here?), we
decrement i which is the "space left" count, but we haven't actually
copied a character.  I think that means that later, we'll end up
padding with the incorrect number of characters.

diff --git a/core/fs/lib/mangle.c b/core/fs/lib/mangle.c
--- a/core/fs/lib/mangle.c
+++ b/core/fs/lib/mangle.c
@@ -23,7 +23,6 @@ void generic_mangle_name(char *dst, const char *src)
         if (*src == '/') {
             if (src[1] == '/') {
                 src++;
-                i--;
                 continue;
             }
         }

Even with these fixes or workarounds in my testing, the ldlinux
searching seems broken.  Specifically, I have my DHCP server configure
to send "/efi/syslinux.efi" as the "filename" and placed ldlinux.e64
in that same directory under the assumption that the documentation is
right and syslinux will look in the same directory as the filename for
ldlinux.EXT.  The code appears to do that, but what the TFTP server
sees is a second request for /efi/syslinux.efi and then ldlinux.e64
searched for in the hard coded search_directories list which doesn't
match my network configuration.

If I change the DHCP configuration to use pathprefix set to /efi/ then
it finds ldlinux.e64 and can find my configfile, but I want to follow
the layout recommended in the wiki documentation where I could have
all three of bios, e32 and e64 versions of com32 modules in
subdirectories of the same parent expecting a PATH directive to find
he right version.  But that  doesn't seem to work either.

TLDR:
If I could get a second pair of eyes on this, I'll turn them into a
series of smaller commits in git and submit a pull request.

Phil P.

-- 
Philip Pokorny, RHCE
Chief Technology Officer
PENGUIN COMPUTING, Inc
www.penguincomputing.com

Changing the world through technical innovation