www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 2838] New: std.file.rmdirRecurse fails

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838

           Summary: std.file.rmdirRecurse fails
           Product: D
           Version: 2.027
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: bugzilla digitalmars.com
        ReportedBy: graham.stjack internode.on.net


The problem seems to be caused by incorrect behaviour of druntime's
core.sys.posix.readdir, which fails to populate the dirent's d_type field - it
is always set to 0.

This causes directories to be identified as files and thus not recursed into.


--
Apr 15 2009
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838






Created an attachment (id=323)
 --> (http://d.puremagic.com/issues/attachment.cgi?id=323&action=view)
Simple test case

The unittest code in std.file will demonstrate the problem, but for what it is
worth, here is my test code. I tracked the problem down as far as printing out
the values passed to std.file.DirEntry.init, and fd.d_type was always zero when
invoked by my test code. I haven't included my hacked version of file.d.


--
Apr 15 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838


unknown simplemachines.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |unknown simplemachines.org





Unable to reproduce on Windows or Linux.  Everything deletes fine.

I should note that readdir() is a libc function, afaik.  It's not a D-specific
function.  If it's not properly filling the structure it's a system problem.

Also note, readdir() is not re-entrant.  There's a readdir_r() for that, which
phobos is not using.  If you are using rmdirRecurse() in a threaded app with
other directory reading happening, that may be the cause of your problems.

-[Unknown]


--
Apr 15 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838






I have looked into it some more, and it looks like setting of d_type is only
supported on some filesystems. I am using reiserfs, which I guess is a bit
unusual, and it could be that.

Yes, I just tried it with a vfat filesystem on a flash-stick, and d_type was
populated ok.

Can I suggest that, since the posix standard makes population of d_type (and
others) optional, we do something to handle the case where it is set to 0
(DT_UNKNOWN)?

Maybe the easiest thing to do is to modify std.file.DirEntry.init to be
something like:

   void init(string path, dirent *fd)
   {
       immutable len = std.c.string.strlen(fd.d_name.ptr);
       name = std.path.join(path, fd.d_name[0 .. len].idup);
       d_type = fd.d_type;
       didstat = false;

       if (d_type == DT_UNKNOWN) {
           ensureStatDone;
       }
   }

or maybe defer it until isdir or isdile are called.

ensureStatDone would need to be modified to be something like:

   void ensureStatDone()
   {
       if (didstat) return;
       enforce(core.sys.posix.sys.stat.stat(toStringz(name), &statbuf) == 0,
               "Failed to stat file `"~name~"'");
       _size = cast(ulong)statbuf.st_size;
       _creationTime = cast(d_time)statbuf.st_ctime * std.date.TicksPerSecond;
       _lastAccessTime = cast(d_time)statbuf.st_atime *
std.date.TicksPerSecond;
       _lastWriteTime = cast(d_time)statbuf.st_mtime * std.date.TicksPerSecond;

       if (d_type == DT_UNKNOWN) {
           if (S_ISDIR(statbuf.st_mode)) {
               d_type = DT_DIR;
           }
           else if (S_ISREG(statbuf.st_mode)) {
               d_type = DT_REG;
           }
       }

       didstat = true;
   }

And of course std.file needs to be changed to use readdir_r too. If this
approach is acceptable to you, I could have a go at submitting a patch if you
like... 


--
Apr 16 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838






Ah, yes, that makes sense.

Please feel free to attach a patch.  I'm not the one who'd accept it, but I
think your approach looks sound.  But, beware of convention - curlies, call
syntax, etc. - it's best to stay common within a file.

-[Unknown]


--
Apr 16 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838






Created an attachment (id=325)
 --> (http://d.puremagic.com/issues/attachment.cgi?id=325&action=view)
Proposed patch for std.file to handle case of linux filesystems that don't set
d_type

Submitted a proposed patch to set DirEntry.d_type in the linux version when the
filesystem doesn't provide d_type is direntry, such as reiserfs.


--
Apr 16 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |andrei metalanguage.com
         AssignedTo|nobody puremagic.com        |andrei metalanguage.com


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 11 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED



15:09:53 PDT ---
http://www.dsource.org/projects/phobos/changeset/2061

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 26 2010
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2838


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmx.com



PDT ---


problem), I added this test to the unittest for dirEntries so that this
situation would be caught:

foreach (DirEntry e; dirEntries("/usr/share/zoneinfo", SpanMode.depth))
{
    assert(e.isfile || e.isdir, e.fullname);
}


I don't know whether or not you think that it would be worth adding such a
test, but it it seems to be a test case which reliably catches the problem.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 26 2010