[phobos] phobos commit, revision 1560

[Adam Ruppe]

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 <rsinfu at gmail.com> wrote:
> Brad Roberts <braddr at puremagic.com> 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
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
>