322183 - Moving a folder on top of a non-empty folder doesn't throw the specified exception on Linux

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jeff Walden [:Waldo]]

If you have an nsIFile for an existing directory and try to moveTo() it on top of an existing, non-empty directory, IDL comments (see URL) say that the return error will be NS_ERROR_FILE_DIR_NOT_EMPTY.  This looks true on OS/2 and Windows, but it's not true on Linux.  (It also probably doesn't works on Macs, because the returned error goes through MacErrorMapper() in nsLocalFile(Mac|OSX).cpp, and there's no case which returns NS_ERROR_FILE_DIR_NOT_EMPTY.)

http://lxr.mozilla.org/mozilla/source/xpcom/io/nsLocalFileUnix.cpp#888

Following the code, we try to use rename(), which fails and sets rv to nsresultForErrno(errno).  That function's just a switch(errno), but there's no case for ENOTEMPTY, so the default NS_ERROR_FAILURE is returned.  The easy solution is to add a case to nsresultForErrno(errno) for ENOTEMPTY to return NS_ERROR_FILE_DIR_NOT_EMPTY.  There'd be a patch immediately following this except that I don't currently have a working Internet connection and a source tree available at the same computer (and might not for a week).  Here's where the change should go, if some reader is able and motivated to make the patch:

http://lxr.mozilla.org/mozilla/source/xpcom/io/nsLocalFile.h#87

nsIFile.remove(false) is also affected by this problem when the file's a non-empty directory on the previously-mentioned platforms with the addition of Windows, and the described patch would fix that return code too.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

Comment 2

[Benjamin Smedberg]

Comment on attachment 207872 [details] [diff] [review]
Patch

I am not an XPCOM peer.

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 207872 [details] [diff] [review]
Patch

Looks good, and I'd say for 1.8 as well.  1.8.0.x is soooo tempting, but it would technically change the semantics of the API (to return a more specific error code), so that might not fly.  I'll try anything, though!

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 207872 [details] [diff] [review]
Patch

not needed for 1.8.0.x

Comment 5

[jag (Peter Annema)]

Apparently AIX didn't like this:

xlC_r -o nsDirectoryService.o -c  -DMOZILLA_INTERNAL_API -DOSTYPE=\"AIX5.1\" -DOSARCH=\"AIX\" -DBUILD_ID=2006033003 -D_IMPL_NS_COM -I.. -I../../dist/include/string -I../../dist/include   -I../../dist/include/xpcom -I../../dist/include/nspr      -qflag=w:w      -DNDEBUG -DTRIMMED -O2 -qmaxmem=-1 -qalias=noansi   -DMOZILLA_CLIENT -D_MOZILLA_CONFIG_H_ -DMOZILLA_VERSION=\"1.9a1\" -DMOZILLA_VERSION_U=1.9a1 -DAIX=1 -DHAVE_SYS_INTTYPES_H=1 -DNSCAP_DISABLE_DEBUG_PTR_TYPES=1 -DD_INO=d_ino -DSTDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGINFO_T=1 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_INT64=1 -DHAVE_UINT=1 -DHAVE_UINT_T=1 -DHAVE_UINT16_T=1 -DHAVE_DIRENT_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_MALLOC_H=1 -DHAVE_X11_XKBLIB_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATFS_H=1 -DHAVE_LIBC_R=1 -DHAVE_LIBM=1 -DHAVE_LIBDL=1 -DHAVE_LIBC_R=1 -DFUNCPROTO=15 -DHAVE_XSHM=1 -D_REENTRANT=1 -DHAVE_RANDOM=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_STAT64=1 -DHAVE_LSTAT64=1 -DHAVE_FLOCKFILE=1 -DHAVE_LOCALTIME_R=1 -DHAVE_STRTOK_R=1 -DHAVE_RES_NINIT=1 -DHAVE_LANGINFO_CODESET=1 -DHAVE_I18N_LC_MESSAGES=1 -DMOZ_DEFAULT_TOOLKIT=\"gtk2\" -DMOZ_WIDGET_GTK2=1 -DMOZ_ENABLE_XREMOTE=1 -DMOZ_X11=1 -DMOZ_SUITE=1 -DMOZ_BUILD_APP=suite -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DMOZ_ENABLE_COREXFONTS=1 -DMOZ_EXTRA_X11CONVERTERS=1 -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_XTF=1 -DMOZ_MATHML=1 -DMOZ_ENABLE_CANVAS=1 -DMOZ_SVG=1 -DMOZ_UPDATE_CHANNEL=default -DMOZ_LOGGING=1 -DMOZ_USER_DIR=\".mozilla\" -DHAVE_INTTYPES_H=1 -DHAVE_UINT64_T=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_MORK=1 -DSUNCTL=1 -DMOZ_DLL_SUFFIX=\".so\" -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DJS_THREADSAFE=1 -DMOZ_ACCESSIBILITY_ATK=1 -DMOZILLA_LOCALE_VERSION=\"1.9a1\" -DMOZILLA_REGION_VERSION=\"1.9a1\" -DMOZILLA_SKIN_VERSION=\"1.8\"  /home/tbox/sb/tinderbox/AIX_5.1_Clobber/mozilla/xpcom/io/nsDirectoryService.cpp
"/home/tbox/sb/tinderbox/AIX_5.1_Clobber/mozilla/xpcom/io/nsLocalFile.h", line 110.7: 1540-0240 (S) A duplicate case value is not allowed.

See also bug 95744.

Comment 6

[Jeff Walden [:Waldo]]

(In reply to comment #5)
> Apparently AIX didn't like this:
> 
> See also bug 95744.

Thanks for the reference; I checked in an #if modeled on the patch there, so now it breaks on whatever was breaking it before.  ;-)  With that, this is fixed on trunk; I'll give it a day or so before checking it in on 1.8.1.

Comment 7

[Jeff Walden [:Waldo]]

Er, wait -- forgot about other platforms like Macs.  Sorry for the bugspam...

Comment 8

[Benjamin Smedberg]

Let's call this FIXED as filed for Linux. The mac code shares the Linux codepath now so it's probably fixed there also.