diff --git a/engine/source/platformX86UNIX/x86UNIXFileio.cc b/engine/source/platformX86UNIX/x86UNIXFileio.cc index bbd470fc3..6fc48d41f 100755 --- a/engine/source/platformX86UNIX/x86UNIXFileio.cc +++ b/engine/source/platformX86UNIX/x86UNIXFileio.cc @@ -137,11 +137,6 @@ return error; } -bool dPathCopy(const char *fromName, const char *toName, bool nooverwrite) -{ - return CopyFile(fromName,toName); -} - //----------------------------------------------------------------------------- static char sgPrefDir[MaxPath]; static bool sgPrefDirInitialized = false; @@ -1249,14 +1244,224 @@ StringTableEntry Platform::osGetTemporaryDirectory() return StringTable->insert("~/"); } +//----------------------------------------------------------------------------- +// Copying, which the editors lean on harder than the name suggests: it is how a +// new project is stamped out of a template, and how a theme is given its own +// copy of the stock cursor art. +// +// Paths are used as handed over, like isFile and isDirectory and fileDelete +// above, rather than routed through MungePath into the pref directory the way +// createPath is. Every caller builds an absolute path first and then asks isFile +// whether the copy arrived, so a copy that landed anywhere else would read as a +// failure -- and MungePath leaves an absolute path alone in any case. +//----------------------------------------------------------------------------- + +static const U32 sCopyBufferSize = 32768; + +// The directories leading up to a file, made on the path exactly as given. +// Platform::createPath is not usable here: it sends a relative path through +// MungePath into the pref directory, which is not where the copy itself would +// then be written, so the open would fail on the parent it had just made +// somewhere else. +static void CreateParentDirectories(const char* path) +{ + char buffer[MaxPath]; + dStrncpy(buffer, path, MaxPath - 1); + buffer[MaxPath - 1] = '\0'; + + for (char* walk = dStrchr(buffer, '/'); walk != NULL; walk = dStrchr(walk + 1, '/')) + { + if (walk == buffer) + continue; // the leading slash of an absolute path + + *walk = '\0'; + mkdir(buffer, 0777); + *walk = '/'; + } +} + +static bool CopyOneFile(const char* fromName, const char* toName, bool nooverwrite) +{ + if (nooverwrite && (Platform::isFile(toName) || Platform::isDirectory(toName))) + return false; + + struct stat fromStat; + if (stat(fromName, &fromStat) < 0) + return false; + + // The destination's folder may not exist yet -- copying a tree creates the + // directories as it walks, but a lone file copied into a new folder does not. + CreateParentDirectories(toName); + + S32 fromFd = open(fromName, O_RDONLY); + if (fromFd < 0) + return false; + + // Carry the mode across, so a copied executable is still executable. + S32 toFd = open(toName, O_WRONLY | O_CREAT | O_TRUNC, fromStat.st_mode & 0777); + if (toFd < 0) + { + close(fromFd); + return false; + } + + char buffer[sCopyBufferSize]; + bool ok = true; + for (;;) + { + const ssize_t got = read(fromFd, buffer, sizeof(buffer)); + if (got == 0) + break; + if (got < 0) + { + if (errno == EINTR) + continue; + ok = false; + break; + } + + ssize_t written = 0; + while (written < got) + { + const ssize_t put = write(toFd, buffer + written, got - written); + if (put < 0) + { + if (errno == EINTR) + continue; + ok = false; + break; + } + written += put; + } + + if (!ok) + break; + } + + close(fromFd); + if (close(toFd) < 0) + ok = false; + + // A half-written file is worse than none: the next run would find it with + // isFile and take it for good art. + if (!ok) + unlink(toName); + + return ok; +} + +static bool CopyOneDirectory(const char* fromName, const char* toName) +{ + DIR* dir = opendir(fromName); + if (dir == NULL) + return false; + + struct stat fromStat; + if (stat(fromName, &fromStat) == 0) + mkdir(toName, fromStat.st_mode & 0777); + else + mkdir(toName, 0700); + + bool ok = true; + struct dirent* entry; + while ((entry = readdir(dir)) != NULL) + { + if (dStrcmp(entry->d_name, ".") == 0 || dStrcmp(entry->d_name, "..") == 0) + continue; + + char fromChild[MaxPath]; + char toChild[MaxPath]; + dSprintf(fromChild, sizeof(fromChild), "%s/%s", fromName, entry->d_name); + dSprintf(toChild, sizeof(toChild), "%s/%s", toName, entry->d_name); + + // Asking the filesystem rather than trusting d_type, which is DT_UNKNOWN + // on filesystems that do not carry the kind in the directory entry. + if (Platform::isDirectory(fromChild)) + { + if (!CopyOneDirectory(fromChild, toChild)) + ok = false; + } + else + { + // Overwriting freely: the caller's nooverwrite was already answered + // against the top of the tree, and stopping here would leave a + // half-copied project behind. + if (!CopyOneFile(fromChild, toChild, false)) + ok = false; + } + } + + closedir(dir); + return ok; +} + bool Platform::pathCopy(const char* source, const char* dest, bool nooverwrite) { + if (source == NULL || dest == NULL || !*source || !*dest) + return false; + + if (Platform::isFile(source)) + return CopyOneFile(source, dest, nooverwrite); + + if (Platform::isDirectory(source)) + { + if (nooverwrite && (Platform::isDirectory(dest) || Platform::isFile(dest))) + return false; + + // Refuse to copy a tree into itself, which would recurse until the path + // outgrew MaxPath. Platform::isSubDirectory is no help here: it matches a + // bare child name against the parent's entries, not one path inside + // another. + dsize_t sourceLen = dStrlen(source); + while (sourceLen > 1 && source[sourceLen - 1] == '/') + sourceLen--; // a trailing slash would put dest past the comparison + + if (dStrncmp(source, dest, sourceLen) == 0 && + (dest[sourceLen] == '/' || dest[sourceLen] == '\0')) + { + Con::errorf("Platform::pathCopy: %s is inside %s", dest, source); + return false; + } + + CreateParentDirectories(dest); + return CopyOneDirectory(source, dest); + } + + Con::errorf("Platform::pathCopy: nothing to copy at %s", source); return false; } bool Platform::fileRename(const char* source, const char* dest) { - return false; + if (source == NULL || dest == NULL || !*source || !*dest) + return false; + + if (!Platform::isFile(source) && !Platform::isDirectory(source)) + { + Con::errorf("Platform::fileRename: no file exists at %s", source); + return false; + } + + if (Platform::isFile(dest) || Platform::isDirectory(dest)) + Con::warnf("Platform::fileRename: overwriting %s", dest); + + CreateParentDirectories(dest); + + if (rename(source, dest) == 0) + return true; + + // rename cannot cross a filesystem, and the pref directory and the game + // directory are not always on the same one. Fall back to moving it by hand. + if (errno != EXDEV) + return false; + + if (!Platform::pathCopy(source, dest, false)) + return false; + + if (Platform::isDirectory(source)) + return Platform::deleteDirectory(source); + + return Platform::fileDelete(source); } bool Platform::fileDelete(const char* name) diff --git a/engine/source/platformX86UNIX/x86UNIXStrings.cc b/engine/source/platformX86UNIX/x86UNIXStrings.cc index 62593a375..6e50f4914 100755 --- a/engine/source/platformX86UNIX/x86UNIXStrings.cc +++ b/engine/source/platformX86UNIX/x86UNIXStrings.cc @@ -133,9 +133,14 @@ char* dStrcatl(char *dst, size_t dstSize, ...) AssertFatal(dstSize > 0, "dStrcatl: destination size is set zero"); dstSize--; // leave room for string termination - // find end of dst - while (dstSize && *p++) - dstSize--; + // find end of dst. Testing *p and stepping separately, because *p++ walks + // past the terminator on the iteration that ends the loop -- which left a + // gap, so "Garage" and "Games" concatenated to "Garage\0Games". + while (dstSize && *p) + { + p++; + dstSize--; + } va_list args; va_start(args, dstSize); @@ -386,7 +391,11 @@ S32 dStrrev(char *str) // Get string length S32 l = dStrlen(str) - 1; - for (int x = 0; x < 1; x++,l--) + // Walking in from both ends until they meet. This read "x < 1", which runs + // a single iteration: "GarageGames" came back as "sarageGameG". Stopping at + // x < l also keeps the XOR swap off the middle character of an odd-length + // string, which it would otherwise zero. + for (int x = 0; x < l; x++,l--) { // triple XOR trick str[x]^=str[l]; @@ -400,19 +409,24 @@ S32 dStrrev(char *str) S32 dItoa(S32 n, char s[]) { - S32 i, sign; + S32 i = 0; + const bool negative = (n < 0); - if ((sign = n) < 0) - n = -n; + // Digits are taken off an unsigned copy: negating S32_MIN does not fit back + // into an S32, and unsigned negation is the one form of it that is defined. + U32 value = negative ? (U32)0 - (U32)n : (U32)n; - i = 0; do { - s[i++] = n % 10 + '0'; - } while((n /= 10) > 0); + s[i++] = (char)(value % 10 + '0'); + } while((value /= 10) > 0); - if (sign < 0) + if (negative) s[i++] = '-'; + // Terminate BEFORE reversing: dStrrev measures with dStrlen, which without + // this ran off the digits into whatever the caller's buffer happened to hold. + s[i] = '\0'; + dStrrev(s); return dStrlen(s); diff --git a/engine/source/testing/tests/platformFileIoTests.cc b/engine/source/testing/tests/platformFileIoTests.cc index 61b9adaf9..bb05fc173 100755 --- a/engine/source/testing/tests/platformFileIoTests.cc +++ b/engine/source/testing/tests/platformFileIoTests.cc @@ -122,6 +122,113 @@ TEST( PlatformFileIOTests, FileWriteRead ) SUCCEED(); } + +//----------------------------------------------------------------------------- +// Copying and renaming. Both were unimplemented stubs returning false on the +// Unix back-end, which is not something a caller can tell from a copy that +// simply did not happen: the editors stamp a new project out of a template and +// give a theme its own cursor art this way, and both silently produced nothing. +// +// Everything here is done against absolute paths built from the working +// directory, because a relative path handed to Platform::createPath is routed +// into the pref directory while the copy itself is written where it was asked +// for -- so a relative test would be checking two different places. +//----------------------------------------------------------------------------- + +#define PLATFORM_UNITTEST_COPY_ROOT "_unitTestCopy_RemoveMe" + +static void unitTestCopyPath( char* buffer, U32 bufferSize, const char* relative ) +{ + dSprintf( buffer, bufferSize, "%s/%s/%s", + Platform::getCurrentDirectory(), PLATFORM_UNITTEST_COPY_ROOT, relative ); +} + +static bool unitTestWriteFile( const char* path, const char* contents ) +{ + File file; + if ( file.open( path, File::Write ) != File::Ok ) + return false; + + U32 written = 0; + const bool ok = file.write( (U32)dStrlen( contents ), contents, &written ) == File::Ok; + file.close(); + return ok; +} + +TEST( PlatformFileIOTests, PathCopyAndRename ) +{ + char root[1024]; + dSprintf( root, sizeof( root ), "%s/%s", + Platform::getCurrentDirectory(), PLATFORM_UNITTEST_COPY_ROOT ); + + // Anything left by a previous run would make the overwrite checks lie. + // Asked only when there is something there, because deleteDirectory + // complains about a directory it cannot read. + if ( Platform::isDirectory( root ) ) + Platform::deleteDirectory( root ); + + char source[1024], target[1024], nested[1024]; + + // --- A single file, into a folder that does not exist yet. --- + unitTestCopyPath( source, sizeof( source ), "src/one.txt" ); + unitTestCopyPath( target, sizeof( target ), "dst/one.txt" ); + + ASSERT_TRUE( unitTestWriteFile( source, "hello" ) ) << "Could not write the source file."; + ASSERT_TRUE( Platform::isFile( source ) ) << "The source file was not created."; + + ASSERT_TRUE( Platform::pathCopy( source, target, true ) ) << "Copying a file failed."; + ASSERT_TRUE( Platform::isFile( target ) ) << "The copy is not there."; + ASSERT_EQ( Platform::getFileSize( source ), Platform::getFileSize( target ) ) << "The copy is a different size."; + + // --- nooverwrite is honoured in both directions. --- + ASSERT_FALSE( Platform::pathCopy( source, target, true ) ) << "Copying over an existing file should be refused."; + ASSERT_TRUE( Platform::pathCopy( source, target, false ) ) << "Copying over an existing file should be allowed when asked."; + + // --- A missing source fails, and leaves nothing behind. --- + unitTestCopyPath( source, sizeof( source ), "src/nosuch.txt" ); + unitTestCopyPath( target, sizeof( target ), "dst/nosuch.txt" ); + ASSERT_FALSE( Platform::pathCopy( source, target, true ) ) << "Copying a file that does not exist should fail."; + ASSERT_FALSE( Platform::isFile( target ) ) << "A failed copy left a file behind."; + + // --- A whole tree, which is how a project is stamped from a template. --- + unitTestCopyPath( source, sizeof( source ), "tree/top.txt" ); + ASSERT_TRUE( unitTestWriteFile( source, "top" ) ) << "Could not write the tree's top file."; + unitTestCopyPath( nested, sizeof( nested ), "tree/deep/deeper/bottom.txt" ); + ASSERT_TRUE( unitTestWriteFile( nested, "bottom" ) ) << "Could not write the tree's nested file."; + + unitTestCopyPath( source, sizeof( source ), "tree" ); + unitTestCopyPath( target, sizeof( target ), "treeCopy" ); + ASSERT_TRUE( Platform::pathCopy( source, target, true ) ) << "Copying a directory failed."; + + unitTestCopyPath( target, sizeof( target ), "treeCopy/top.txt" ); + ASSERT_TRUE( Platform::isFile( target ) ) << "The tree's top file did not come across."; + unitTestCopyPath( target, sizeof( target ), "treeCopy/deep/deeper/bottom.txt" ); + ASSERT_TRUE( Platform::isFile( target ) ) << "The tree's nested file did not come across."; + unitTestCopyPath( target, sizeof( target ), "treeCopy/deep/deeper" ); + ASSERT_TRUE( Platform::isDirectory( target ) ) << "The tree's nested folder did not come across."; + + // --- A tree copied into itself would recurse until the path ran out. --- + unitTestCopyPath( source, sizeof( source ), "tree" ); + unitTestCopyPath( target, sizeof( target ), "tree/inner" ); + ASSERT_FALSE( Platform::pathCopy( source, target, true ) ) << "A directory should not be copied into itself."; + + // --- Renaming, which has no script binding and so is only reachable here. --- + unitTestCopyPath( source, sizeof( source ), "dst/one.txt" ); + unitTestCopyPath( target, sizeof( target ), "dst/two.txt" ); + ASSERT_TRUE( Platform::fileRename( source, target ) ) << "Renaming a file failed."; + ASSERT_TRUE( Platform::isFile( target ) ) << "The renamed file is not at its new name."; + ASSERT_FALSE( Platform::isFile( source ) ) << "The renamed file is still at its old name."; + + unitTestCopyPath( source, sizeof( source ), "dst/nosuch.txt" ); + unitTestCopyPath( target, sizeof( target ), "dst/three.txt" ); + ASSERT_FALSE( Platform::fileRename( source, target ) ) << "Renaming a file that does not exist should fail."; + + // Tidy up, and check the cleanup itself worked so the next run starts clean. + ASSERT_TRUE( Platform::deleteDirectory( root ) ) << "Could not remove the test directory."; + ASSERT_FALSE( Platform::isDirectory( root ) ) << "The test directory is still there."; + + SUCCEED(); +} //----------------------------------------------------------------------------- #endif // TORQUE_SHIPPING