[commit] r1875 - in trunk/GME: Core Gme
GMESRC Repository Notifications
gme-commit at list.isis.vanderbilt.edu
Thu Mar 22 15:23:19 CDT 2012
Author: ksmyth
Date: Thu Mar 22 15:23:18 2012
New Revision: 1875
Log:
Fix bug where CloseHandle on the .mga file would allow it to be modified (despite the mapping being read-only). The mapping could then read bogus data.
Modified:
trunk/GME/Core/CoreBinFile.cpp
trunk/GME/Core/CoreBinFile.h
trunk/GME/Gme/GMEApp.cpp
Modified: trunk/GME/Core/CoreBinFile.cpp
==============================================================================
--- trunk/GME/Core/CoreBinFile.cpp Thu Mar 22 15:23:06 2012 (r1874)
+++ trunk/GME/Core/CoreBinFile.cpp Thu Mar 22 15:23:18 2012 (r1875)
@@ -970,22 +970,42 @@
objects.clear();
}
-void CCoreBinFile::SaveProject()
+static DWORD __stdcall prog(LARGE_INTEGER TotalFileSize, LARGE_INTEGER TotalBytesTransferred,LARGE_INTEGER StreamSize,
+LARGE_INTEGER StreamBytesTransferred, DWORD dwStreamNumber, DWORD dwCallbackReason, HANDLE hSourceFile, HANDLE hDestinationFile, LPVOID lpData)
+{
+ return PROGRESS_STOP;
+}
+
+
+void CCoreBinFile::SaveProject(const std::string& origfname, bool keepoldname)
{
ASSERT( !ofs.is_open() );
ASSERT( metaprojectid.size() == 16 );
- // FIXME: KMS: it could happen that the save fails, and the user loses data. (at least this isn't worse than previous versions)
- HANDLE hFile = CreateFileA(filename.c_str(), GENERIC_READ, 0, NULL, OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, NULL);
- CloseHandle(hFile);
+ BOOL cancel = FALSE;
+
+ std::string filenameout = filename;
+ // origfname == filename => file_buffer has filename locked FILE_SHARE_READ
+ // CopyFile because:
+ // SetEndOfFileInformationFile
+ // Preserves extended attributes, NTFS alternate streams, file attributes (and newer Windows: security attributes)
+ if (origfname == filename)
+ {
+ filenameout += "tmp";
+ BOOL succ = CopyFileExA(origfname.c_str(), filenameout.c_str(), &prog, NULL, &cancel, 0);
+ if (!succ && GetLastError() != ERROR_REQUEST_ABORTED)
+ HR_THROW(HRESULT_FROM_WIN32(GetLastError()));
+ }
+ // TODO:
+ // GetNamedSecurityInfo(source, GROUP_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION)
+ // SetNamedSecurityInfo(target, GROUP_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION)
ofs.clear();
- ofs.open(filename.c_str(), std::ios::out | std::ios::binary);
+ ofs.open(filenameout.c_str(), std::ios::out | std::ios::binary);
if( ofs.fail() || !ofs.is_open() ) {
ofs.close();
ofs.clear();
- // FIXME: can we HRESULT_FROM_WIN32(GetLastError) here instead?
- HR_THROW(E_FILEOPEN);
+ HR_THROW(HRESULT_FROM_WIN32(GetLastError()));
}
write(metaprojectid);
@@ -1015,6 +1035,25 @@
HR_THROW(E_FILEOPEN);
ofs.close();
+
+ file_buffer.~membuf();
+ new ((void*)&file_buffer) membuf();
+
+ if (origfname == filename)
+ {
+ BOOL succ = MoveFileExA(filenameout.c_str(), filename.c_str(), MOVEFILE_REPLACE_EXISTING);
+ if (!succ)
+ {
+ //CloseProject(VARIANT_TRUE);
+ HR_THROW(HRESULT_FROM_WIN32(GetLastError()));
+ }
+ }
+
+ if (file_buffer.open((keepoldname ? origfname : filename).c_str()) != 0) {
+ HR_THROW(HRESULT_FROM_WIN32(GetLastError()));
+ }
+ cifs = file_buffer.getBegin();
+ cifs_eof = file_buffer.getEnd();
}
void CCoreBinFile::LoadProject()
@@ -1216,9 +1255,11 @@
filename = fn;
if(filename.empty()) filename = ".";
}
- if(filename == ".") COMTHROW(E_NAMEMISSING);
- SaveProject();
- if(keepoldname == VARIANT_TRUE) filename = origfname;
+ if (filename == ".")
+ COMTHROW(E_NAMEMISSING);
+ SaveProject(origfname, keepoldname != VARIANT_FALSE);
+ if (keepoldname != VARIANT_FALSE)
+ filename = origfname;
}
COMCATCH( filename = origfname;)
}
Modified: trunk/GME/Core/CoreBinFile.h
==============================================================================
--- trunk/GME/Core/CoreBinFile.h Thu Mar 22 15:23:06 2012 (r1874)
+++ trunk/GME/Core/CoreBinFile.h Thu Mar 22 15:23:18 2012 (r1875)
@@ -19,7 +19,8 @@
{ }
int open(const char* filename) {
- hFile = CreateFileA(filename, GENERIC_READ, FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_READONLY, NULL);
+ ASSERT(hFile == INVALID_HANDLE_VALUE);
+ hFile = CreateFileA(filename, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_READONLY, NULL);
if (hFile == INVALID_HANDLE_VALUE) {
return 1;
}
@@ -36,10 +37,6 @@
return 1;
}
end = begin + filesize;
- CloseHandle(hFile);
- hFile = INVALID_HANDLE_VALUE;
- CloseHandle(hFileMappingObject);
- hFileMappingObject = INVALID_HANDLE_VALUE;
return 0;
}
@@ -334,7 +331,7 @@
bool InTransaction() const { return intrans; }
void CancelProject() NOTHROW;
- void SaveProject();
+ void SaveProject(const std::string& origfname, bool keepoldname);
void LoadProject();
public:
@@ -429,11 +426,12 @@
CopyTo(a, p);
}
virtual void Write(CCoreBinFile *binfile) const {
- if (pos == NULL) {
- binfile->write(a);
- } else {
- binfile->writestring(pos);
+ if (pos != NULL) {
+ BinAttr<VALTYPE_STRING>* this_ = const_cast<BinAttr<VALTYPE_STRING>*>(this);
+ binfile->read(this_->a, this_->pos);
+ this_->pos = NULL;
}
+ binfile->write(a);
}
virtual void Read(CCoreBinFile *binfile) {
binfile->readstring(pos);
@@ -511,18 +509,31 @@
}
virtual void Write(CCoreBinFile *binfile) const {
if (data)
+ {
+ if (!need_free)
+ {
+ // need to get data off the disk; the file is going away
+ BinAttr<VALTYPE_BINARY>* this_ = const_cast<BinAttr<VALTYPE_BINARY>*>(this);
+ unsigned char* olddata = this_->data;
+ int len = this->len;
+ this_->data = (unsigned char*)malloc(sizeof(len) + len);
+ this_->need_free = true;
+ this_->len = len;
+ memcpy(value, olddata+sizeof(len), len);
+ }
binfile->write(value, len);
+ }
else
binfile->write((unsigned char*)NULL, 0);
}
virtual void Read(CCoreBinFile *binfile) {
data = (unsigned char*)binfile->cifs;
- // to test without lazy read:
int len = this->len;
- data = (unsigned char*)malloc(sizeof(len) + len);
- need_free = true;
- this->len = len;
- memcpy(value, binfile->cifs+sizeof(len), len);
+ // to test without lazy read:
+ //data = (unsigned char*)malloc(sizeof(len) + len);
+ //need_free = true;
+ //this->len = len;
+ //memcpy(value, binfile->cifs+sizeof(len), len);
binfile->cifs += sizeof(len) + len;
}
BinAttr(BinAttr<VALTYPE_BINARY>&& that) : BinAttrBase(that.attrid), data(that.data), need_free(that.need_free) { that.need_free = false; }
@@ -598,8 +609,7 @@
if (dict == NULL)
{
- // need to read before write
- // TODO: could just blit it
+ // need to read before write, since the file is going away
CComVariant p;
Get(binfile, &p);
}
Modified: trunk/GME/Gme/GMEApp.cpp
==============================================================================
--- trunk/GME/Gme/GMEApp.cpp Thu Mar 22 15:23:06 2012 (r1874)
+++ trunk/GME/Gme/GMEApp.cpp Thu Mar 22 15:23:18 2012 (r1875)
@@ -1544,7 +1544,17 @@
if( mgaProject != NULL ) {
HRESULT hr = mgaProject->Save(CComBSTR(conn), VARIANT_FALSE);
if(hr != S_OK) {
- AfxMessageBox(_T("ERROR: Could not save project\nCheck access permissions"));
+ CComBSTR error;
+ if (GetErrorInfo(&error)) {
+ CString errmsg = _T("Could not save project: ");
+ errmsg += error;
+ if (hr == HRESULT_FROM_WIN32(ERROR_ACCESS_DENIED)) {
+ errmsg += _T("\nCheck that no other GME has this file open");
+ }
+ AfxMessageBox(errmsg);
+ } else {
+ AfxMessageBox(_T("ERROR: Could not save project\nCheck access permissions"));
+ }
}
}
More information about the gme-commit
mailing list