From c140217d25efa1168299d069abe2a9eda10eeabc Mon Sep 17 00:00:00 2001 From: monarch dodra Date: Wed, 10 Jul 2013 17:19:39 +0200 Subject: [PATCH 1/4] Fix Issue 10607 - DirEntry has no constructor http://d.puremagic.com/issues/show_bug.cgi?id=10607 Fixes: auto a = DirEntry("path"); //Correctly works Simplified code a little bit (Axed _init) Improved FileException usage --- std/file.d | 206 +++++++++++++++++++++++++++-------------------------- 1 file changed, 106 insertions(+), 100 deletions(-) diff --git a/std/file.d b/std/file.d index c1ddb870de1..ea959b6ae5f 100644 --- a/std/file.d +++ b/std/file.d @@ -53,7 +53,6 @@ version (unittest) _first = false; } - return _deleteme; } } @@ -1528,7 +1527,7 @@ else version(Posix) string readLink(C)(const(C)[] link) dynamicBuffer.length = dynamicBuffer.length * 3 / 2; } - throw new FileException(format("Path for %s is too long to read.", link)); + throw new FileException(text(link), "Path is too long to read."); } version(Posix) unittest @@ -1605,8 +1604,16 @@ version(StdDdoc) +/ struct DirEntry { - void _init(T...)(T); - public: + /++ + Constructs a DirEntry for the given file (or directory). + + Params: + path = The file (or directory) to get a DirEntry for. + + Throws: + $(D FileException) if the file does not exist. + +/ + this(string path); /++ Returns the path to the file represented by this $(D DirEntry). @@ -1750,6 +1757,52 @@ else version(Windows) public: alias name this; + this(string path) + { + if(!path.exists) + throw new FileException(path, text("File does not exist")); + + _name = path; + + with (getFileAttributesWin(path)) + { + _size = makeUlong(nFileSizeLow, nFileSizeHigh); + _timeCreated = std.datetime.FILETIMEToSysTime(&ftCreationTime); + _timeLastAccessed = std.datetime.FILETIMEToSysTime(&ftLastAccessTime); + _timeLastModified = std.datetime.FILETIMEToSysTime(&ftLastWriteTime); + _attributes = dwFileAttributes; + } + } + + private this(string path, in WIN32_FIND_DATA* fd) + { + auto clength = to!int(core.stdc.string.strlen(fd.cFileName.ptr)); + + // Convert cFileName[] to unicode + const wlength = MultiByteToWideChar(0, 0, fd.cFileName.ptr, clength, null, 0); + auto wbuf = new wchar[wlength]; + const n = MultiByteToWideChar(0, 0, fd.cFileName.ptr, clength, wbuf.ptr, wlength); + assert(n == wlength); + // toUTF8() returns a new buffer + _name = buildPath(path, std.utf.toUTF8(wbuf[0 .. wlength])); + _size = (cast(ulong)fd.nFileSizeHigh << 32) | fd.nFileSizeLow; + _timeCreated = std.datetime.FILETIMEToSysTime(&fd.ftCreationTime); + _timeLastAccessed = std.datetime.FILETIMEToSysTime(&fd.ftLastAccessTime); + _timeLastModified = std.datetime.FILETIMEToSysTime(&fd.ftLastWriteTime); + _attributes = fd.dwFileAttributes; + } + private this(string path, in WIN32_FIND_DATAW *fd) + { + size_t clength = std.string.wcslen(fd.cFileName.ptr); + _name = std.utf.toUTF8(fd.cFileName[0 .. clength]); + _name = buildPath(path, std.utf.toUTF8(fd.cFileName[0 .. clength])); + _size = (cast(ulong)fd.nFileSizeHigh << 32) | fd.nFileSizeLow; + _timeCreated = std.datetime.FILETIMEToSysTime(&fd.ftCreationTime); + _timeLastAccessed = std.datetime.FILETIMEToSysTime(&fd.ftLastAccessTime); + _timeLastModified = std.datetime.FILETIMEToSysTime(&fd.ftLastWriteTime); + _attributes = fd.dwFileAttributes; + } + @property string name() const pure nothrow { return _name; @@ -1804,55 +1857,8 @@ else version(Windows) } private: - - void _init(in char[] path) - { - _name = path.idup; - - with (getFileAttributesWin(path)) - { - _size = makeUlong(nFileSizeLow, nFileSizeHigh); - _timeCreated = std.datetime.FILETIMEToSysTime(&ftCreationTime); - _timeLastAccessed = std.datetime.FILETIMEToSysTime(&ftLastAccessTime); - _timeLastModified = std.datetime.FILETIMEToSysTime(&ftLastWriteTime); - _attributes = dwFileAttributes; - } - } - - void _init(in char[] path, in WIN32_FIND_DATA* fd) - { - auto clength = to!int(core.stdc.string.strlen(fd.cFileName.ptr)); - - // Convert cFileName[] to unicode - const wlength = MultiByteToWideChar(0, 0, fd.cFileName.ptr, clength, null, 0); - auto wbuf = new wchar[wlength]; - const n = MultiByteToWideChar(0, 0, fd.cFileName.ptr, clength, wbuf.ptr, wlength); - assert(n == wlength); - // toUTF8() returns a new buffer - _name = buildPath(path, std.utf.toUTF8(wbuf[0 .. wlength])); - _size = (cast(ulong)fd.nFileSizeHigh << 32) | fd.nFileSizeLow; - _timeCreated = std.datetime.FILETIMEToSysTime(&fd.ftCreationTime); - _timeLastAccessed = std.datetime.FILETIMEToSysTime(&fd.ftLastAccessTime); - _timeLastModified = std.datetime.FILETIMEToSysTime(&fd.ftLastWriteTime); - _attributes = fd.dwFileAttributes; - } - - void _init(in char[] path, in WIN32_FIND_DATAW *fd) - { - size_t clength = std.string.wcslen(fd.cFileName.ptr); - _name = std.utf.toUTF8(fd.cFileName[0 .. clength]); - _name = buildPath(path, std.utf.toUTF8(fd.cFileName[0 .. clength])); - _size = (cast(ulong)fd.nFileSizeHigh << 32) | fd.nFileSizeLow; - _timeCreated = std.datetime.FILETIMEToSysTime(&fd.ftCreationTime); - _timeLastAccessed = std.datetime.FILETIMEToSysTime(&fd.ftLastAccessTime); - _timeLastModified = std.datetime.FILETIMEToSysTime(&fd.ftLastWriteTime); - _attributes = fd.dwFileAttributes; - } - - string _name; /// The file or directory represented by this DirEntry. - SysTime _timeCreated; /// The time when the file was created. SysTime _timeLastAccessed; /// The time when the file was last accessed. SysTime _timeLastModified; /// The time when the file was last modified. @@ -1868,6 +1874,43 @@ else version(Posix) public: alias name this; + this(string path) + { + if(!path.exists) + throw new FileException(path, text("File does not exist")); + + _name = path; + + _didLStat = false; + _didStat = false; + _dTypeSet = false; + } + + private this(string path, core.sys.posix.dirent.dirent* fd) + { + immutable len = core.stdc.string.strlen(fd.d_name.ptr); + _name = buildPath(path, fd.d_name[0 .. len]); + + _didLStat = false; + _didStat = false; + + //fd_d_type doesn't work for all file systems, + //in which case the result is DT_UNKOWN. But we + //can determine the correct type from lstat, so + //we'll only set the dtype here if we could + //correctly determine it (not lstat in the case + //of DT_UNKNOWN in case we don't ever actually + //need the dtype, thus potentially avoiding the + //cost of calling lstat). + if(fd.d_type != DT_UNKNOWN) + { + _dType = fd.d_type; + _dTypeSet = true; + } + else + _dTypeSet = false; + } + @property string name() const pure nothrow { return _name; @@ -1943,41 +1986,6 @@ else version(Posix) } private: - - void _init(in char[] path) - { - _name = path.idup; - - _didLStat = false; - _didStat = false; - _dTypeSet = false; - } - - void _init(in char[] path, core.sys.posix.dirent.dirent* fd) - { - immutable len = core.stdc.string.strlen(fd.d_name.ptr); - _name = buildPath(path, fd.d_name[0 .. len]); - - _didLStat = false; - _didStat = false; - - //fd_d_type doesn't work for all file systems, - //in which case the result is DT_UNKOWN. But we - //can determine the correct type from lstat, so - //we'll only set the dtype here if we could - //correctly determine it (not lstat in the case - //of DT_UNKNOWN in case we don't ever actually - //need the dtype, thus potentially avoiding the - //cost of calling lstat). - if(fd.d_type != DT_UNKNOWN) - { - _dType = fd.d_type; - _dTypeSet = true; - } - else - _dTypeSet = false; - } - /++ This is to support lazy evaluation, because doing stat's is expensive and not always needed. @@ -2013,7 +2021,6 @@ else version(Posix) _didLStat = true; } - string _name; /// The file or directory represented by this DirEntry. stat_t _statBuf = void; /// The result of stat(). @@ -2172,7 +2179,8 @@ unittest +/ void rmdirRecurse(in char[] pathname) { - DirEntry de = dirEntry(pathname); + //DirEntry never exists this scope, so cast to string is safe + DirEntry de = dirEntry(cast(string)pathname); rmdirRecurse(de); } @@ -2189,7 +2197,7 @@ void rmdirRecurse(in char[] pathname) void rmdirRecurse(ref DirEntry de) { if(!de.isDir) - throw new FileException(text("File ", de.name, " is not a directory")); + throw new FileException(de.name, "Not a directory"); if(de.isSymlink) remove(de.name); @@ -2338,7 +2346,7 @@ private struct DirIteratorImpl popDirStack(); return false; } - _cur._init(_stack.data[$-1].dirpath, findinfo); + _cur = DirEntry(_stack.data[$-1].dirpath, findinfo); return true; } @@ -2359,7 +2367,7 @@ private struct DirIteratorImpl popDirStack(); return false; } - _cur._init(_stack.data[$-1].dirpath, findinfo); + _cur = DirEntry(_stack.data[$-1].dirpath, findinfo); return true; } @@ -2406,7 +2414,7 @@ private struct DirIteratorImpl if(core.stdc.string.strcmp(fdata.d_name.ptr, ".") && core.stdc.string.strcmp(fdata.d_name.ptr, "..") ) { - _cur._init(_stack.data[$-1].dirpath, fdata); + _cur = DirEntry(_stack.data[$-1].dirpath, fdata); return true; } } @@ -2661,16 +2669,14 @@ auto dirEntries(string path, string pattern, SpanMode mode, Throws: $(D FileException) if the file does not exist. +/ +DirEntry dirEntry(string name) +{ + return DirEntry(name); +} +deprecated("dirEntry taking a mutable string is deprecated. Please use `dirEntry(name.idup)` instead.") DirEntry dirEntry(in char[] name) { - if(!name.exists) - throw new FileException(text("File ", name, " does not exist")); - - DirEntry dirEntry; - - dirEntry._init(name); - - return dirEntry; + return dirEntry(name.idup); } //Test dirEntry with a directory. From c9f91c21e7a80a0bd74d92a15601fd7b73624514 Mon Sep 17 00:00:00 2001 From: monarch dodra Date: Fri, 12 Jul 2013 14:18:48 +0200 Subject: [PATCH 2/4] Fixing some un-necessary calls to text. Fix comment. --- std/file.d | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/std/file.d b/std/file.d index ea959b6ae5f..7d49b104923 100644 --- a/std/file.d +++ b/std/file.d @@ -1527,7 +1527,7 @@ else version(Posix) string readLink(C)(const(C)[] link) dynamicBuffer.length = dynamicBuffer.length * 3 / 2; } - throw new FileException(text(link), "Path is too long to read."); + throw new FileException(to!string(link), "Path is too long to read."); } version(Posix) unittest @@ -1760,7 +1760,7 @@ else version(Windows) this(string path) { if(!path.exists) - throw new FileException(path, text("File does not exist")); + throw new FileException(path, "File does not exist"); _name = path; @@ -1877,7 +1877,7 @@ else version(Posix) this(string path) { if(!path.exists) - throw new FileException(path, text("File does not exist")); + throw new FileException(path, "File does not exist"); _name = path; @@ -2179,7 +2179,8 @@ unittest +/ void rmdirRecurse(in char[] pathname) { - //DirEntry never exists this scope, so cast to string is safe + //No references to pathname will be kept after rmdirRecurse, + //so the cast is safe DirEntry de = dirEntry(cast(string)pathname); rmdirRecurse(de); From f7b417192a335fa037ea7e66d37372ddd8d36ac5 Mon Sep 17 00:00:00 2001 From: monarch dodra Date: Fri, 12 Jul 2013 15:14:07 +0200 Subject: [PATCH 3/4] Fix Issue 10621 - dirEntry is (now) useless --- std/file.d | 56 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/std/file.d b/std/file.d index 7d49b104923..5ceafe79e08 100644 --- a/std/file.d +++ b/std/file.d @@ -1080,7 +1080,7 @@ unittest possible for both $(D isFile) and $(D isDir) to be $(D false) for a particular file (in which case, it's a special file). You can use $(D getAttributes) to get the attributes to figure out what type of special - it is, or you can use $(D dirEntry) to get at its $(D statBuf), which is the + it is, or you can use $(D DirEntry) to get at its $(D statBuf), which is the result from $(D stat). In either case, see the man page for $(D stat) for more information. @@ -1597,10 +1597,6 @@ version(StdDdoc) { /++ Info on a file, similar to what you'd get from stat on a Posix system. - - A $(D DirEntry) is obtained by using the functions $(D dirEntry) (to get - the $(D DirEntry) for a specific file) or $(D dirEntries) (to get a - $(D DirEntry) for each file/directory in a particular directory). +/ struct DirEntry { @@ -1620,10 +1616,10 @@ version(StdDdoc) Examples: -------------------- -auto de1 = dirEntry("/etc/fonts/fonts.conf"); +auto de1 = DirEntry("/etc/fonts/fonts.conf"); assert(de1.name == "/etc/fonts/fonts.conf"); -auto de2 = dirEntry("/usr/share/include"); +auto de2 = DirEntry("/usr/share/include"); assert(de2.name == "/usr/share/include"); -------------------- +/ @@ -1636,10 +1632,10 @@ assert(de2.name == "/usr/share/include"); Examples: -------------------- -auto de1 = dirEntry("/etc/fonts/fonts.conf"); +auto de1 = DirEntry("/etc/fonts/fonts.conf"); assert(!de1.isDir); -auto de2 = dirEntry("/usr/share/include"); +auto de2 = DirEntry("/usr/share/include"); assert(de2.isDir); -------------------- +/ @@ -1662,10 +1658,10 @@ assert(de2.isDir); Examples: -------------------- -auto de1 = dirEntry("/etc/fonts/fonts.conf"); +auto de1 = DirEntry("/etc/fonts/fonts.conf"); assert(de1.isFile); -auto de2 = dirEntry("/usr/share/include"); +auto de2 = DirEntry("/usr/share/include"); assert(!de2.isFile); -------------------- +/ @@ -2039,7 +2035,7 @@ unittest { if("C:\\Program Files\\".exists) { - auto de = dirEntry("C:\\Program Files\\"); + auto de = DirEntry("C:\\Program Files\\"); assert(!de.isFile); assert(de.isDir); assert(!de.isSymlink); @@ -2047,13 +2043,13 @@ unittest if("C:\\Users\\".exists && "C:\\Documents and Settings\\".exists) { - auto de = dirEntry("C:\\Documents and Settings\\"); + auto de = DirEntry("C:\\Documents and Settings\\"); assert(de.isSymlink); } if("C:\\Windows\\system.ini".exists) { - auto de = dirEntry("C:\\Windows\\system.ini"); + auto de = DirEntry("C:\\Windows\\system.ini"); assert(de.isFile); assert(!de.isDir); assert(!de.isSymlink); @@ -2064,7 +2060,7 @@ unittest if("/usr/include".exists) { { - auto de = dirEntry("/usr/include"); + auto de = DirEntry("/usr/include"); assert(!de.isFile); assert(de.isDir); assert(!de.isSymlink); @@ -2076,7 +2072,7 @@ unittest core.sys.posix.unistd.symlink("/usr/include", symfile.ptr); { - auto de = dirEntry(symfile); + auto de = DirEntry(symfile); assert(!de.isFile); assert(de.isDir); assert(de.isSymlink); @@ -2085,7 +2081,7 @@ unittest if("/usr/include/assert.h".exists) { - auto de = dirEntry("/usr/include/assert.h"); + auto de = DirEntry("/usr/include/assert.h"); assert(de.isFile); assert(!de.isDir); assert(!de.isSymlink); @@ -2181,12 +2177,9 @@ void rmdirRecurse(in char[] pathname) { //No references to pathname will be kept after rmdirRecurse, //so the cast is safe - DirEntry de = dirEntry(cast(string)pathname); - - rmdirRecurse(de); + rmdirRecurse(DirEntry(cast(string)pathname)); } - /++ Remove directory and all of its content and subdirectories, recursively. @@ -2214,6 +2207,16 @@ void rmdirRecurse(ref DirEntry de) rmdir(de.name); } } +///ditto +//Note, without this overload, passing an RValue DirEntry still works, but +//actually fully reconstructs a DirEntry inside the +//"rmdirRecurse(in char[] pathname)" implementation. That is needlessly +//expensive. +//A DirEntry is a bit big (72B), so keeping the "by ref" signature is desirable. +void rmdirRecurse(DirEntry de) +{ + rmdirRecurse(de); +} version(Windows) unittest { @@ -2662,6 +2665,9 @@ auto dirEntries(string path, string pattern, SpanMode mode, } /++ + $(RED Deprecated. It will be removed in July 2014. + Please use $(LREF DirEntry) constructor directly instead.) + Returns a DirEntry for the given file (or directory). Params: @@ -2670,14 +2676,10 @@ auto dirEntries(string path, string pattern, SpanMode mode, Throws: $(D FileException) if the file does not exist. +/ -DirEntry dirEntry(string name) -{ - return DirEntry(name); -} -deprecated("dirEntry taking a mutable string is deprecated. Please use `dirEntry(name.idup)` instead.") +deprecated("Please use DirEntry constructor directly instead.") DirEntry dirEntry(in char[] name) { - return dirEntry(name.idup); + return DirEntry(name.idup); } //Test dirEntry with a directory. From af2f38856979894ccba7133bf3bca37f836528d7 Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 13 Jul 2013 12:39:57 +0200 Subject: [PATCH 4/4] Fix typo in xformat doc --- std/string.d | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/std/string.d b/std/string.d index d5dca635b4f..4196e128d89 100644 --- a/std/string.d +++ b/std/string.d @@ -2465,7 +2465,7 @@ unittest /***************************************************** * $(RED Deprecated. It will be removed in November 2013. - * Please us std.string.format instead) + * Please use std.string.format instead.) * * Format arguments into a string. * @@ -2493,8 +2493,8 @@ deprecated unittest /***************************************************** - * $(RED Deprecated. It will be removed in November 2013). - * Please us std.string.sformat instead) + * $(RED Deprecated. It will be removed in November 2013. + * Please use std.string.sformat instead.) * * Format arguments into string $(D buf) which must be large * enough to hold the result. Throws RangeError if it is not.