[phobos] phobos commit, revision 1560

[Shin Fujishiro]

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