Syslinux - Nov 2018 - I heard the patch window was open? Two small patches

I see that there seems to be patches being accepted...

Please consider committing these two:

You should be able to cherry pick them from my syslinux repo at
http://github.com/ppokorny/syslinux I would think?

commit 64fb71b87a625d70ea8c6127f18fe96d3f4fdc89
Author: Philip Pokorny <ppokorny at penguincomputing.com>
Date:   Sun Mar 5 21:07:48 2017 -0800

    Fix PATH parsing to match the documentation at
    http://www.syslinux.org/wiki/index.php?title=Config#PATH
    that the list is space-separated

diff --git a/com32/elflink/ldlinux/readconfig.c
b/com32/elflink/ldlinux/readconfig.c
index 3d6aa27..c6b86d4 100644
--- 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);


commit 647e30a1d12ec8a71551910e06e1d16fca27851e
Author: Philip Pokorny <ppokorny at penguincomputing.com>
Date:   Sun Mar 5 20:59:05 2017 -0800

    Change path_add so that it adds new entries at the end not
    the beginning of the list.  This fixes an issue where PATH
    with multiple directories listed would effectively reverse
    the order of the searched directories

diff --git a/core/path.c b/core/path.c
index 8e517ca..7b4d886 100644
--- 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;



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

Changing the world through technical innovation

>     Fix PATH parsing to match the documentation at
>     http://www.syslinux.org/wiki/index.php?title=Config#PATH
>     that the list is space-separated
> 
> 
>     Change path_add so that it adds new entries at the end not
>     the beginning of the list.  This fixes an issue where PATH
>     with multiple directories listed would effectively reverse
>     the order of the searched directories
> 
 
The original email thread about the task of the ":" symbol in/for the 
PATH directive was started during 2013Jun, around the same time version 
5.10 was released ("PXE + dhcp opts 209, 210 and path issues in 
tftp/http"). Version 5.11-pre1 was released just for this matter.


The recommendation for users should be:
1_ avoid multiple paths for the PATH directive;
2_ if you "must" use multiple paths for the PATH directive, use 
multiple statements with one path per row, instead of using multiple 
paths within one PATH directive row/statement.
3_ There has been very little discussion - almost none at all - about 
the practical utility / purpose of having multiple paths in the PATH 
directive.

Having said that, the aforementioned email thread ("PXE + dhcp opts 
209, 210 and path issues in tftp/http") shows that using ":" as 
separator for the PATH directive can be problematic even without making 
use of the PATH directive (at all). Quoting from Matt Fleming:

 In retrospect, choosing the colon character as the entry separator for
 the PATH directive was not a smart move, as that character is also 
used
 in TFTP-style paths. This conflict manifests as PXELINUX being unable 
to
 find and load files.

So, this is (was?) a problem anyway.

An important detail to note is that Matt mentioned in the mailing list 
the intention (at some point of the discussion) to use the space 
character instead of ":" in the proposed patch, but the text was 
stripped (down/out) from the actual commit. See:

 https://www.syslinux.org/archives/2013-June/020131.html 

versus the _text_ itself in the commit:

 
https://repo.or.cz/syslinux.git/commit/19455794d144c2c38f483890e0d384bc4
3b5fd63 



But that commit is not exactly the proposed patch that was supposed to 
change the separator:

 [PATCH] PATH: Change the PATH directive syntax 

from Matt is seen at:
 https://www.syslinux.org/archives/2013-June/020138.html 

Quote: "Switch to using the space character to separate PATH entries"

One reason for the discrepancy is probably that the feedback for that 
proposed patch was that it failed:
 https://www.syslinux.org/archives/2013-June/020143.html

Quote:
 Nope :( I tested using this fixed patch, but still not work 
 

Perhaps one reason the "space character" change was not actually 
committed in any way or form can be explained by:

 https://www.syslinux.org/archives/2013-June/020146.html 

Quote:
 I'll split this patch up before committing anything 


There was also the matter of allowing space characters in paths (from 
HPA, same email thread). Since the specific problem presented in that 
email thread was (or, seemed to be) solved (without changing the ":"),
then perhaps the matter of the specific character was (silently) 
postponed?


So, there seems to be one commit changing the PATH directive to be a 
linked list, but the use of ":" was not changed (?). Since the
feedback
was that the linked list solved the "ftp-like" paths problem, perhaps 
Matt decided that the ":" should prevail, instead of changing it, but 
he never actually said anything?



The PATH directive has too-many inconsistencies (e.g. absolute paths 
vs. relative paths), bugs, unsupported cases... And, from the point of 
view of users, Syslinux should provide better alternatives (which were 
partly-proposed publicly in the mailing list too, even by Matt himself, 
and myself too).

Unfortunately, this matter has been ignored / postponed for a very long 
time 
already.

Best Regards,
Ady.