I remember seeing what looked like a bug in remove() when working on the Windows port of rdmd. I don't know if it is related to what you just changed.
I think I got the problem figured out, but I never committed a fix. First, create a file and remove it. Should be no exception, worked last time I saw it. Next, create a file, open it in another process (I used a directory and cmd.exe), then remove it. Now, this is where I saw it throw an exception saying "success". I figure the problem was that the Windows function returned something saying "file in use, but delete is queued up", which enforce saw as an error, but GetLastError called success. Probably a different bug than what you looked at, but maybe not. On 5/27/10, Shin Fujishiro <[email protected]> wrote: > Brad Roberts <[email protected]> wrote: >> On 5/26/2010 7:21 AM, dsource.org wrote: >> > phobos commit, revision 1560 >> > >> > >> > user: rsinfu >> > >> > msg: >> > Fixed bugzilla 4188: std.file.remove throws Exception on success. >> > >> > cenforce() used getErrno(), not GetLastError(), for errors happened in >> > Win32 API. >> > >> > http://www.dsource.org/projects/phobos/changeset/1560 >> >> This change seems wrong. >> >> Is there a use of cenforce that's wrong? Does the inclusion of getErrno >> need to >> be conditional on something? >> >> Pure removal of the call to getErrno removes important error text that's >> useful >> in many contexts. >> >> - Brad > > It's correct. getErrno (GetLastError) is automatically called as a > default argument. > > Let's look inside the fixed cenforce(): > -------------------- > private T cenforce(T, string file = __FILE__, uint line = __LINE__) > (T condition, lazy const(char)[] name) > { > if (!condition) > { > throw new FileException( > text("In ", file, "(", line, "), data file ", name)); > } > return condition; > } > -------------------- > It throws FileException with a single string argument. The called > constructor is this: > -------------------- > class FileException : Exception > { > ... > version(Windows) this(string name, uint errno = GetLastError) > { > this(name, sysErrorString(errno)); > this.errno = errno; > } > version(Posix) this(in char[] name, uint errno = .getErrno) > { > auto s = strerror(errno); > this(name, to!string(s)); > this.errno = errno; > } > } > -------------------- > Note the default argument. It utilizes appropriate error number on the > compiled platform, and the constructor converts it to error message. > So, the pure removal of getErrno works. Or rather, passing getErrno() > is definitely wrong on Windows. > > Actually, FileExceptions are thrown in this way in other places of > std/file.d. For example: > -------------------- > version(Windows) void rename(in char[] from, in char[] to) > { > enforce(useWfuncs > ? MoveFileW(std.utf.toUTF16z(from), std.utf.toUTF16z(to)) > : MoveFileA(toMBSz(from), toMBSz(to)), > new FileException( > text("Attempting to rename file ", from, " to ", > to))); > } > -------------------- > > I verified it, tested the code on Posix and Win32. I got correct > message on error. Then I commited the code to the repository. > > I'm sorry if my commit message caused you to misunderstand. The commit > message might be too short; I should have explained these things in the > commit message. > > > Shin > _______________________________________________ > phobos mailing list > [email protected] > http://lists.puremagic.com/mailman/listinfo/phobos > _______________________________________________ phobos mailing list [email protected] http://lists.puremagic.com/mailman/listinfo/phobos
