Re: mingw remove bug

[Jim Meyering]

Eric Blake wrote:
> Eric Blake <ebb9 <at> byu.net> writes:
>> > I'm arguing that the second program should also report "No such
>> > file or directory".
>>
>> Ah, so for 'foo/', the code should distinguish between ENOENT and ENOTDIR,
>> based on whether 'foo' exists.  I'll update the patch and test accordingly.
>
> I'm taking a step back.  Rather than my first attempt at rpl_remove, it seems
> like I should be able to get away with much simpler code, provided that the
> underlying syscalls aren't broken.  It certainly has the benefit of avoiding 
> an
> unnecessary [l]stat:
>
> int result = rmdir (name);
> if (result == -1 && errno == ENOTDIR)
>   result = unlink (name);
> return result;
>
> (glibc remove() does it the other way around: attempting unlink first and only
> calling rmdir if errno == EISDIR/EPERM.  This works fine on Linux, but is too
> dangerous on platforms where unlink can remove a directory)
>
> So, I found two reasons that the rmdir module should no longer be obsolete -
> cygwin 1.5.x mistakenly removes a directory given with a trailing "./", even
> though POSIX requires this to fail with EINVAL; and mingw rmdir("file/") fails
> with EINVAL instead of ENOTDIR (which would leak EINVAL out of the above
> rpl_remove() logic, instead of confirming ENOTDIR failure with 
> unlink("file/")).
>
> Meanwhile, the rmdir-errno module, in use by coreutils until today, guessed
> wrong for cross-compilation to Solaris (where rmdir fails with EEXIST, not
> ENOTEMPTY, on a populated directory); now that coreutils no longer uses the
> module [1], I see no reason to keep the module alive.

I was thinking of marking it as obsolete,
(AFAIK, there's been no actual problem report)
but if coreutils was the only user, deleting should be ok.

> [1] http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/18181/focus=18182
>
> By the way, this patch series also fixes unlinkat(-1,"/dir/./",AT_REMOVEDIR) 
> on
> cygwin 1.5.x, since the openat module already depended on rmdir.  So far, I
> don't know of any systems with native unlinkat but with broken rmdir, but I
> suppose I should add tests for unlinkat to verify the same behavior.
>
> Technically, GNU/Linux is not POSIX-compliant: rmdir("symlink-to-empty-dir/")
> fails with ENOTDIR, instead of succeeding at turning symlink-to-empty-dir into
> a dangling link.  But this behavior is counter-intuitive (both rmdir("symlink-
> to-file/") and rmdir("symlink-to-full-dir/") are required to fail, so why is
> symlink-to-empty-dir special?), and GNU/Linux is at least consistent (rename
> ("symlink-to-empty-dir/","other") also has non-POSIX behavior to prevent
> accidental dangling symlink creation).

I think I made some changes to keep the tools sane even on certain
offending systems.  IMHO, applying tools like rm, rmdir, mv, or rename
to symlink-to-dir/ should never create a dangling symlink.

> So I intentionally don't want to
> penalize GNU/Linux with a rmdir(2) wrapper that creates dangling symlinks; if
> we want rm(1) and rmdir(1) to do so under POSIXLY_CORRECT, then they can
> perform the extra checks manually.
>
> Here's what I plan on pushing later today.
>
>
> From: Eric Blake <address@hidden>
> Date: Wed, 16 Sep 2009 10:08:55 -0600
> Subject: [PATCH 1/2] rmdir: work around cygwin 1.5.x and mingw bugs
>
> * m4/rmdir.m4 (gl_FUNC_RMDIR): Detect the bugs.
> * lib/rmdir.c (rmdir): Work around it.
> * modules/rmdir (Status, Notice): No longer obsolete.
> (Files): Add dos.m4.
> (Depends-on): Add unistd.
> (configure.ac): Set witnesses.
> (License): Relax to LGPLv2+.
> * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Set defaults.
> * modules/unistd (Makefile.am): Substitute witnesses.
> * lib/unistd.in.h (rmdir): Declare replacement.
> * doc/posix-functions/rmdir.texi (rmdir): Document this.
> * modules/rmdir-tests: New tests.
> * tests/test-rmdir.c: Likewise.

Sounds good.