Re: [PATCH] Code cleanup in load_module

[Jose E. Marchesi]

Hi Tim.

    2020-04-08  Tim Rühsen  <address@hidden>
    
            * lib/pkl-tab.y (load_module): Code cleanup.
    ---
     ChangeLog     |  4 ++++
     lib/pkl-tab.y | 46 ++++++++++++++++------------------------------
     2 files changed, 20 insertions(+), 30 deletions(-)
    
    diff --git a/lib/pkl-tab.y b/lib/pkl-tab.y
    index 6a8f17f5..22708f46 100644
    --- a/lib/pkl-tab.y
    +++ b/lib/pkl-tab.y
    @@ -224,41 +224,27 @@ load_module (struct pkl_parser *parser,
          requested module.  */
       {
         char *full_filename;
    -    char *saveptr = NULL;
    -    char *path = xstrdup (load_path); /* Modified by strtok.  */
    -    char *dir = strtok_r (path, ":", &saveptr);
    +    const char *s, *e;
    
    -    if (dir)
    +    for (s = load_path, e = s; *e; s = e + 1)
           {
    -        do
    +        /* Ignore empty entries. */
    +        if ((e = strchrnul (s, ':')) == s)
    +          continue;
    +
    +        if (!strncmp (s, "%DATADIR%", e - s))
    +          full_filename = pk_str_concat (PKGDATADIR, "/", filename, NULL);

Only now I realize that we are not doing the right thing for situations
where %DATADIR% has some trailing stuff before the next ':' or '\0',
like in:

   :%DATADIR%foo:

If I'm not wrong in both the previous implementation and this new one
`foo' would be ignored.

We could not do the replacement in that case, but it would be nice to
support something like:

  :%DATADIR%/mysubdir:

What about something like:


if (!strncmp (s, "%DATADIR%", e - s))
   full_filename = pk_str_concat (PKGDATADIR, "/", filename,
                                  s + strlen ("%DATADIR%"),
                                  NULL);