On Fri, 16 Sep 2016 10:15:17 +0200 Tim Ruehsen <tim.rueh...@gmx.de> 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 <tim.rueh...@gmx.de> 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 <gscriv...@gnu.org> wrote: > > > > > Hi Matthew, > > > > > > > > > > Matthew White <mehw.is...@inventati.org> 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 <mehw.is...@inventati.org>
pgp3qWLNhgO7A.pgp
Description: PGP signature