[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Meta
From: |
Matthew White |
Subject: |
Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml |
Date: |
Fri, 16 Sep 2016 17:15:17 +0200 |
On Fri, 16 Sep 2016 10:15:17 +0200
Tim Ruehsen <address@hidden> wrote:
> On Thursday, September 15, 2016 7:56:45 PM CEST Matthew White wrote:
> > 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?]
>
> Just make sure that no file name beginning with letter+colon is used for
> system
> calls on Windows (e.g. open("C:D:file/E:F:new", ...) is not a good idea).
> Either you strip the 'C:D:', or percent escape ':' on Windows. Wget has
> functions to percent escape special characters in file names, depending on
> the
> OS it is built on.
Thanks. Function amended to always remove prefix drive letters if required
(i.e. on w32 environments) for safety reasons.
About %-escaping, in src/url.c there is 'static void append_uri_pathel()'. I
can't find references to opt.restrict_files_os in other places (except
src/init.c and src/options.h).
Could be useful to write an extern function to do file name %-escaping?
The reason of this function is to replace an existing basename with another
basename. Is it appropriate to do a file name verification here?
The results in the table below regards w32 environments:
*name + ref -> result
------------------------------------------------
NULL + "foo/C:D:file" -> "file"
"foobar" + "foo/C:D:file" -> "file"
"dir/old" + "foo/C:D:file" -> "dir/C:D:file" [w32 invalid*]
"C:D:////E:F:dir/old" + "foo/G:H:new" -> "dir/G:H:new" [w32 invalid*]
"C:D:////E:F:/old" + "foo/G:H:new" -> "new"
* See the post by Eli
http://lists.gnu.org/archive/html/bug-wget/2016-09/msg00103.html .
void
replace_metalink_basename (char **name, char *ref)
{
int n;
char *p, *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);
/* Replace the old basename. */
new = aprintf ("%s%s", *name ? *name : "", ref ? ref : "");
xfree (*name);
*name = new;
/* Remove prefix drive letters if required, i.e. when in w32
environments. */
p = new;
while (p[0] != '\0')
{
while ((n = FILE_SYSTEM_PREFIX_LEN (p)))
p += n;
if (p != new)
{
while (ISSLASH (p[0]))
++p;
new = p;
continue;
}
break;
}
if (*name != new)
{
new = xstrdup (new);
xfree (*name);
*name = new;
}
}
>
> Regards, Tim
Regards,
Matthew
--
Matthew White <address@hidden>
pgpjWf4a9V2Ge.pgp
Description: PGP signature
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, (continued)
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Giuseppe Scrivano, 2016/09/14
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Matthew White, 2016/09/14
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Giuseppe Scrivano, 2016/09/15
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Matthew White, 2016/09/15
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Tim Ruehsen, 2016/09/15
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Matthew White, 2016/09/15
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Tim Ruehsen, 2016/09/16
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Eli Zaretskii, 2016/09/16
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Tim Ruehsen, 2016/09/16
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml, Eli Zaretskii, 2016/09/16
- Re: [Bug-wget] [PATCH 20/25] New option --metalink-index to process Metalink application/metalink4+xml,
Matthew White <=