Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml

[Matthew White]

On Thu, 15 Sep 2016 16:48:01 +0200
Tim Ruehsen <address@hidden> wrote:

> On Thursday, September 15, 2016 4:16:44 PM CEST Matthew White wrote:
> > On Thu, 15 Sep 2016 09:11:31 +0200
> > 
> > Giuseppe Scrivano <address@hidden> wrote:
> > > Hi Matthew,
> > > 
> > > Matthew White <address@hidden> writes:
> > > >> > Function amended to modify *name in place.
> > > >> > 
> > > >> > Followed Tim's suggestions for Patch 09/25 about different
> > > >> > environments compatibility, the function now uses last_component()
> > > >> > to detect the basename:
> > > >> > http://lists.gnu.org/archive/html/bug-wget/2016-09/msg00083.html
> > > >> > 
> > > >> > NOTES: if *name is NULL and ref is like 'dir/C:D:file', the result
> > > >> > will be 'C:D:file'; is it advisable to remove the drive letters
> > > >> > 'C:D:' and return only 'file'?
> > > >> > 
> > > >> > /*
> > > >> > 
> > > >> >   Replace/remove the basename of a file name.
> > > >> >   
> > > >> >   The file name is permanently modified.
> > > >> >   
> > > >> >   Always set NAME to a string, even an empty one.
> > > >> >   
> > > >> >   Use REF's basename as replacement.  If REF is NULL or if it doesn't
> > > >> >   provide a valid basename candidate, then remove NAME's basename.
> > > >> > 
> > > >> > */
> > > >> > void
> > > >> > replace_metalink_basename (char **name, char *ref)
> > > >> > {
> > > >> 
> > > >> is it something we could do using only dirname and basename?  What you
> > > >> need here is "dirname(name) + basename(ref)"?
> > > > 
> > > > You asked to avoid superfluous memory allocations, right?
> > > 
> > > yes, and that is still my idea.  I was just wondering if the cost of
> > > these extra memory allocations was worth.  Is it enough to use
> > > last_component() to get it working on Windows?
> > 
> > Extra memory allocations shouldn't be a concern any longer, they are removed
> > in the amended function definition previously posted:
> > http://lists.gnu.org/archive/html/bug-wget/2016-09/msg00094.html
> > 
> > About base_name() and last_component() multi-environment compatibility:
> > * lib/basename.c (base_name): Call last_component() to find the basename,
> > allocate the basename (prefix with './' if required) * lib/basename-lgpl.c
> > (last_component): Use the macros FILE_SYSTEM_PREFIX_LEN and ISSLASH to
> > isolate the basename * lib/dosname.h: Define the macros
> > FILE_SYSTEM_PREFIX_LEN and ISSLASH to work on different system environments
> > 
> > Forgive this copy and paste, but my question about
> > replace_metalink_basename() is "if *name is NULL and ref is like
> > 'dir/C:D:file', the result will be 'C:D:file'; is it advisable to remove
> > the drive letters 'C:D:' and return only 'file'?".
> 
> Yes, if you use the result as file name to any file/system calls.

Function amended to remove prefix drive letters (aka FILE_SYSTEM_PREFIX_LEN) 
from the resulting file name, when the file name is a bare basename.

Use the dirname from *name and the basename from ref to compute the resulting 
file name.

  *name          +  ref           -> result
  -----------------------------------------
  NULL           + "foo/C:D:file" -> "file"             [bare basename]
  "foobar"       + "foo/C:D:file" -> "file"             [bare basename]
  "dir/old"      + "foo/C:D:file" -> "dir/C:D:file"
  "C:D:file/old" + "foo/E:F:new"  -> "C:D:file/E:F:new" [is this ok?] 

void
replace_metalink_basename (char **name, char *ref)
{
  int n;
  char *new, *basename;

  if (!name)
    return;

  /* Strip old basename.  */
  if (*name)
    {
      basename = last_component (*name);

      if (basename == *name)
        xfree (*name);
      else
        *basename = '\0';
    }

  /* New basename from file name reference.  */
  if (ref)
    {
      ref = last_component (ref);

      if (!*name)
        while ((n = FILE_SYSTEM_PREFIX_LEN (ref)))
          ref += n;
    }

  /* Replace the old basename.  */
  new = aprintf ("%s%s", *name ? *name : "", ref ? ref : "");
  xfree (*name);
  *name = new;
}

> 
> > PS: I'm currently working to create a common python class for the Metalink
> > tests (testenv), see Patch 01/25
> 
> :thumbs up:

All right!

> 
> Regards, Tim

Regards,
Matthew

-- 
Matthew White <address@hidden>

pgpTNT2zeYe_F.pgp
Description: PGP signature