From 9a9510d9e34c507a7d9a24f9cc9a111659d2b151 Mon Sep 17 00:00:00 2001 From: wmayer Date: Tue, 7 May 2013 22:42:58 +0200 Subject: [PATCH] Make file read-only once assigned to a PropertyFileIncluded instance --- src/App/Document.cpp | 2 +- src/App/PropertyFile.cpp | 128 ++++++++++++++++++++++++++++++--------- src/App/PropertyFile.h | 1 + src/Base/FileInfo.cpp | 57 +++++++++++++---- src/Base/FileInfo.h | 10 ++- 5 files changed, 155 insertions(+), 43 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index fb9f28a37..3707e5193 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -607,7 +607,7 @@ Document::~Document() // Remark: The API of Py::Object has been changed to set whether the wrapper owns the passed // Python object or not. In the constructor we forced the wrapper to own the object so we need // not to dec'ref the Python object any more. - // But we must still invalidate the Python object because it need not to be + // But we must still invalidate the Python object because it doesn't need to be // destructed right now because the interpreter can own several references to it. Base::PyObjectBase* doc = (Base::PyObjectBase*)DocumentPythonObject.ptr(); // Call before decrementing the reference counter, otherwise a heap error can occur diff --git a/src/App/PropertyFile.cpp b/src/App/PropertyFile.cpp index 65fbca44c..221ad82c3 100644 --- a/src/App/PropertyFile.cpp +++ b/src/App/PropertyFile.cpp @@ -42,7 +42,7 @@ #include "Document.h" #include "PropertyContainer.h" #include "DocumentObject.h" -#define new DEBUG_CLIENTBLOCK + using namespace App; using namespace Base; using namespace std; @@ -66,10 +66,25 @@ PropertyFileIncluded::~PropertyFileIncluded() // clean up if (!_cValue.empty()) { Base::FileInfo file(_cValue.c_str()); + file.setPermissions(Base::FileInfo::ReadWrite); file.deleteFile(); } } +void PropertyFileIncluded::aboutToSetValue(void) +{ + // This is a trick to check in Copy() if it is called + // directly from outside or by the Undo/Redo mechanism. + // In the latter case it is sufficient to rename the file + // because another file will be assigned afterwards. + // If Copy() is directly called (e.g. to copy the file to + // another document) a copy of the file needs to be created. + // This copy will be deleted again in the class destructor. + this->StatusBits.set(10); + Property::aboutToSetValue(); + this->StatusBits.reset(10); +} + std::string PropertyFileIncluded::getDocTransientPath(void) const { std::string path; @@ -118,8 +133,10 @@ void PropertyFileIncluded::setValue(const char* sFile, const char* sName) // remove old file (if not moved by undo) Base::FileInfo value(_cValue); std::string pathAct = value.dirPath(); - if (value.exists()) + if (value.exists()) { + value.setPermissions(Base::FileInfo::ReadWrite); value.deleteFile(); + } // if a special name given, use this instead if (sName) { @@ -153,25 +170,29 @@ void PropertyFileIncluded::setValue(const char* sFile, const char* sName) _BaseFileName = file.fileName(); } - // That's wrong and can lead to loss of data!!! - // Just consider the example that two objects with this property - // exist in the same document and as an initial step the data are - // copied from one object to the other. A rename will cause the one - // object to loose its data. -#if 0 + // The following applies only on files that are inside the transient + // directory: + // When a file is read-only it is supposed to be assigned to a + // PropertyFileIncluded instance. In this case we must copy the + // file because otherwise the above instance looses its data. + // If the file is writable it is supposed to be of free use and + // it can be simply renamed. + // if the file is already in transient dir of the document, just use it - if (path == pathTrans) { + if (path == pathTrans && file.isWritable()) { bool done = file.renameFile(_cValue.c_str()); if (!done) { std::stringstream str; str << "Cannot rename file " << file.filePath() << " to " << _cValue; throw Base::Exception(str.str()); } + + // make the file read-only + Base::FileInfo dst(_cValue); + dst.setPermissions(Base::FileInfo::ReadOnly); } // otherwise copy from origin location - else -#endif - { + else { // if file already exists in transient dir make a new unique name Base::FileInfo fi(_cValue); if (fi.exists()) { @@ -200,6 +221,10 @@ void PropertyFileIncluded::setValue(const char* sFile, const char* sName) str << "Cannot copy file from " << file.filePath() << " to " << _cValue; throw Base::Exception(str.str()); } + + // make the file read-only + Base::FileInfo dst(_cValue); + dst.setPermissions(Base::FileInfo::ReadOnly); } hasSetValue(); @@ -350,6 +375,9 @@ void PropertyFileIncluded::Restore(Base::XMLReader &reader) reader.readBinFile(_cValue.c_str()); reader.readEndElement("FileIncluded"); _BaseFileName = file; + // set read-only after restoring the file + Base::FileInfo fi(_cValue.c_str()); + fi.setPermissions(Base::FileInfo::ReadOnly); hasSetValue(); } } @@ -375,7 +403,8 @@ void PropertyFileIncluded::SaveDocFile (Base::Writer &writer) const void PropertyFileIncluded::RestoreDocFile(Base::Reader &reader) { - Base::ofstream to(Base::FileInfo(_cValue.c_str())); + Base::FileInfo fi(_cValue.c_str()); + Base::ofstream to(fi); if (!to) { std::stringstream str; str << "PropertyFileIncluded::RestoreDocFile(): " @@ -390,6 +419,9 @@ void PropertyFileIncluded::RestoreDocFile(Base::Reader &reader) to.put((const char)c); } to.close(); + + // set read-only after restoring the file + fi.setPermissions(Base::FileInfo::ReadOnly); hasSetValue(); } @@ -404,19 +436,35 @@ Property *PropertyFileIncluded::Copy(void) const if (file.exists()) { // create a new name in the document transient directory Base::FileInfo newName(getUniqueFileName(file.dirPath(), file.fileName())); - // copy the file - bool done = file.copyTo(newName.filePath().c_str()); - if (!done) { - std::stringstream str; - str << "PropertyFileIncluded::Copy(): " - << "Copying the file '" << file.filePath() << "' to '" - << newName.filePath() << "' failed."; - throw Base::Exception(str.str()); + if (this->StatusBits.test(10)) { + // rename the file + bool done = file.renameFile(newName.filePath().c_str()); + if (!done) { + std::stringstream str; + str << "PropertyFileIncluded::Copy(): " + << "Renaming the file '" << file.filePath() << "' to '" + << newName.filePath() << "' failed."; + throw Base::Exception(str.str()); + } + } + else { + // copy the file + bool done = file.copyTo(newName.filePath().c_str()); + if (!done) { + std::stringstream str; + str << "PropertyFileIncluded::Copy(): " + << "Copying the file '" << file.filePath() << "' to '" + << newName.filePath() << "' failed."; + throw Base::Exception(str.str()); + } } // remember the new name for the Undo Base::Console().Log("Copy '%s' to '%s'\n",_cValue.c_str(),newName.filePath().c_str()); prop->_cValue = newName.filePath().c_str(); + + // make backup files writable to avoid copying them again on undo/redo + newName.setPermissions(Base::FileInfo::ReadWrite); } return prop; @@ -429,22 +477,42 @@ void PropertyFileIncluded::Paste(const Property &from) // make sure that source and destination file are different if (_cValue != prop._cValue) { // delete old file (if still there) - Base::FileInfo(_cValue).deleteFile(); + Base::FileInfo fi(_cValue); + fi.setPermissions(Base::FileInfo::ReadWrite); + fi.deleteFile(); // get path to destination which can be the transient directory // of another document - std::string path = getDocTransientPath(); + std::string pathTrans = getDocTransientPath(); Base::FileInfo fiSrc(prop._cValue); - Base::FileInfo fiDst(path + "/" + prop._BaseFileName); + Base::FileInfo fiDst(pathTrans + "/" + prop._BaseFileName); + std::string path = fiSrc.dirPath(); + if (fiSrc.exists()) { fiDst.setFile(getUniqueFileName(fiDst.dirPath(), fiDst.fileName())); - if (!fiSrc.copyTo(fiDst.filePath().c_str())) { - std::stringstream str; - str << "PropertyFileIncluded::Paste(): " - << "Copying the file '" << fiSrc.filePath() << "' to '" - << fiDst.filePath() << "' failed."; - throw Base::Exception(str.str()); + + // if the file is already in transient dir of the document, just use it + if (path == pathTrans) { + if (!fiSrc.renameFile(fiDst.filePath().c_str())) { + std::stringstream str; + str << "PropertyFileIncluded::Paste(): " + << "Renaming the file '" << fiSrc.filePath() << "' to '" + << fiDst.filePath() << "' failed."; + throw Base::Exception(str.str()); + } } + else { + if (!fiSrc.copyTo(fiDst.filePath().c_str())) { + std::stringstream str; + str << "PropertyFileIncluded::Paste(): " + << "Copying the file '" << fiSrc.filePath() << "' to '" + << fiDst.filePath() << "' failed."; + throw Base::Exception(str.str()); + } + } + + // set the file again read-only + fiDst.setPermissions(Base::FileInfo::ReadOnly); _cValue = fiDst.filePath(); } else { diff --git a/src/App/PropertyFile.h b/src/App/PropertyFile.h index 9aff9b258..d69f93148 100644 --- a/src/App/PropertyFile.h +++ b/src/App/PropertyFile.h @@ -108,6 +108,7 @@ protected: // get the transient path if the property is in a DocumentObject std::string getDocTransientPath(void) const; std::string getUniqueFileName(const std::string&, const std::string&) const; + void aboutToSetValue(void); protected: mutable std::string _cValue; diff --git a/src/Base/FileInfo.cpp b/src/Base/FileInfo.cpp index 9577cd05f..6e8119f0c 100644 --- a/src/Base/FileInfo.cpp +++ b/src/Base/FileInfo.cpp @@ -52,10 +52,20 @@ #include #include -#define new DEBUG_CLIENTBLOCK - using namespace Base; +#ifndef R_OK +#define R_OK 4 /* Test for read permission */ +#endif +#ifndef W_OK +#define W_OK 2 /* Test for write permission */ +#endif +#ifndef X_OK +#define X_OK 1 /* Test for execute permission */ +#endif +#ifndef F_OK +#define F_OK 0 /* Test for existence */ +#endif //********************************************************************************** // helper @@ -263,9 +273,9 @@ bool FileInfo::exists () const { #if defined (FC_OS_WIN32) std::wstring wstr = toStdWString(); - return _waccess(wstr.c_str(),0) == 0; + return _waccess(wstr.c_str(),F_OK) == 0; #elif defined (FC_OS_LINUX) || defined(FC_OS_CYGWIN) || defined(FC_OS_MACOSX) || defined(FC_OS_BSD) - return access(FileName.c_str(),0) == 0; + return access(FileName.c_str(),F_OK) == 0; #endif } @@ -273,9 +283,9 @@ bool FileInfo::isReadable () const { #if defined (FC_OS_WIN32) std::wstring wstr = toStdWString(); - return _waccess(wstr.c_str(),4) == 0; + return _waccess(wstr.c_str(),R_OK) == 0; #elif defined (FC_OS_LINUX) || defined(FC_OS_CYGWIN) || defined(FC_OS_MACOSX) || defined(FC_OS_BSD) - return access(FileName.c_str(),4) == 0; + return access(FileName.c_str(),R_OK) == 0; #endif } @@ -283,9 +293,29 @@ bool FileInfo::isWritable () const { #if defined (FC_OS_WIN32) std::wstring wstr = toStdWString(); - return _waccess(wstr.c_str(),2) == 0; + return _waccess(wstr.c_str(),W_OK) == 0; #elif defined (FC_OS_LINUX) || defined(FC_OS_CYGWIN) || defined(FC_OS_MACOSX) || defined(FC_OS_BSD) - return access(FileName.c_str(),2) == 0; + return access(FileName.c_str(),W_OK) == 0; +#endif +} + +bool FileInfo::setPermissions (Permissions perms) +{ + bool ret = false; + int mode = 0; + + if (perms & FileInfo::ReadOnly) + mode |= S_IREAD; + if (perms & FileInfo::WriteOnly) + mode |= S_IWRITE; + + if (mode == 0) // bad argument + return false; +#if defined (FC_OS_WIN32) + std::wstring wstr = toStdWString(); + return _wchmod(wstr.c_str(),mode) == 0; +#elif defined (FC_OS_LINUX) || defined(FC_OS_CYGWIN) || defined(FC_OS_MACOSX) || defined(FC_OS_BSD) + return chmod(FileName.c_str(),mode) == 0; #endif } @@ -473,12 +503,17 @@ bool FileInfo::deleteDirectoryRecursive(void) const std::vector List = getDirectoryContent(); for (std::vector::iterator It = List.begin();It!=List.end();++It) { - if (It->isDir()) + if (It->isDir()) { + It->setPermissions(FileInfo::ReadWrite); It->deleteDirectoryRecursive(); - else if(It->isFile()) + } + else if (It->isFile()) { + It->setPermissions(FileInfo::ReadWrite); It->deleteFile(); - else + } + else { Base::Exception("FileInfo::deleteDirectoryRecursive(): Unknown object Type in directory!"); + } } return deleteDirectory(); } diff --git a/src/Base/FileInfo.h b/src/Base/FileInfo.h index f541dc216..52fd77548 100644 --- a/src/Base/FileInfo.h +++ b/src/Base/FileInfo.h @@ -42,6 +42,12 @@ namespace Base class BaseExport FileInfo { public: + enum Permissions { + WriteOnly = 0x01, + ReadOnly = 0x02, + ReadWrite = 0x03, + }; + /// Constrction FileInfo (const char* _FileName=""); FileInfo (const std::string &_FileName); @@ -89,6 +95,8 @@ public: bool isReadable () const; /// Checks if the file exist and is writable bool isWritable () const; + /// Tries to set the file permisson + bool setPermissions (Permissions); /// Checks if it is a file (not a direrctory) bool isFile () const; /// Checks if it is a directory (not a file) @@ -109,7 +117,7 @@ public: std::vector getDirectoryContent(void) const; /// Delete an empty directory bool deleteDirectory(void) const; - /// Delete a directory and all its content + /// Delete a directory and all its content. bool deleteDirectoryRecursive(void) const; //@}