From edb7c131205ddf529128480b8bbcc1ce7562b5c7 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Sun, 23 Nov 2014 10:34:57 -0800 Subject: [PATCH] Fix diskimg OpenImage The OpenImage method had an overload that took void*. This turns out to be a bad idea, because void* matches any pointer type that didn't match something else. So the WCHAR* filenames were going to the "open from buffer" method rather than the "open from file" variant. A less important issue is whether open-from-buffer should take a const or non-const pointer. If the "readOnly" boolean flag is not set, then the contents can be altered and const is inappropriate. The best course seems to be to drop the boolean flag as an argument, and just have two different methods. --- app/Actions.cpp | 4 +++- app/DiskArchive.cpp | 7 +++++-- app/Tools.cpp | 18 ++++++++++++------ app/VolumeCopyDialog.cpp | 3 ++- diskimg/DiskImg.cpp | 16 ++++++++++++---- diskimg/DiskImg.h | 5 ++++- diskimg/GenericFD.cpp | 2 +- 7 files changed, 39 insertions(+), 16 deletions(-) diff --git a/app/Actions.cpp b/app/Actions.cpp index 1ff5670..0dfe2b1 100644 --- a/app/Actions.cpp +++ b/app/Actions.cpp @@ -301,6 +301,7 @@ void MainWindow::OnActionsAddDisks(void) DiskImg img; CString failed, errMsg; CString openFilters, saveFolder; + CStringA pathNameA; AddFilesDialog addOpts; LOGI("Add disks!"); @@ -325,7 +326,8 @@ void MainWindow::OnActionsAddDisks(void) fPreferences.SetPrefString(kPrAddFileFolder, saveFolder); /* open the image file and analyze it */ - dierr = img.OpenImage(dlg.GetPathName(), PathProposal::kLocalFssep, true); + pathNameA = dlg.GetPathName(); + dierr = img.OpenImage(pathNameA, PathProposal::kLocalFssep, true); if (dierr != kDIErrNone) { errMsg.Format(L"Unable to open disk image: %hs.", DiskImgLib::DIStrError(dierr)); diff --git a/app/DiskArchive.cpp b/app/DiskArchive.cpp index b981cbd..8e426aa 100644 --- a/app/DiskArchive.cpp +++ b/app/DiskArchive.cpp @@ -466,13 +466,16 @@ GenericArchive::OpenResult DiskArchive::Open(const WCHAR* filename, { CWaitCursor waitc; - dierr = fDiskImg.OpenImage(filename, PathProposal::kLocalFssep, readOnly); + CStringA fileNameA(filename); + dierr = fDiskImg.OpenImage(fileNameA, PathProposal::kLocalFssep, + readOnly); if (dierr == kDIErrAccessDenied && !readOnly && !isVolume) { // retry file open with read-only set // don't do that for volumes -- assume they know what they want LOGI(" Retrying open with read-only set"); fIsReadOnly = readOnly = true; - dierr = fDiskImg.OpenImage(filename, PathProposal::kLocalFssep, readOnly); + dierr = fDiskImg.OpenImage(fileNameA, PathProposal::kLocalFssep, + readOnly); } if (dierr != kDIErrNone) { if (dierr == kDIErrFileArchive) diff --git a/app/Tools.cpp b/app/Tools.cpp index bd0e685..420b6aa 100644 --- a/app/Tools.cpp +++ b/app/Tools.cpp @@ -167,7 +167,8 @@ void MainWindow::OnToolsDiskEdit(void) { CWaitCursor waitc; /* open the image file and analyze it */ - dierr = img.OpenImage(loadName, PathProposal::kLocalFssep, true); + CStringA loadNameA(loadName); + dierr = img.OpenImage(loadNameA, PathProposal::kLocalFssep, true); } #else /* quick test of memory-buffer-based interface */ @@ -343,7 +344,7 @@ void MainWindow::OnToolsDiskConv(void) DiskImg srcImg, dstImg; DiskConvertDialog convDlg(this); CString storageName; - CStringA saveNameA, storageNameA; + CStringA saveNameA, storageNameA, loadNameA; /* flush current archive in case that's what we're planning to convert */ OnFileSave(); @@ -372,7 +373,8 @@ void MainWindow::OnToolsDiskConv(void) fPreferences.SetPrefString(kPrOpenArchiveFolder, saveFolder); /* open the image file and analyze it */ - dierr = srcImg.OpenImage(loadName, PathProposal::kLocalFssep, true); + loadNameA = loadName; + dierr = srcImg.OpenImage(loadNameA, PathProposal::kLocalFssep, true); if (dierr != kDIErrNone) { errMsg.Format(L"Unable to open disk image: %hs.", DiskImgLib::DIStrError(dierr)); @@ -1214,7 +1216,8 @@ void MainWindow::BulkConvertImage(const WCHAR* pathName, const WCHAR* targetDir, fPreferences.GetPrefLong(kPrCompressionType)); /* open the image file and analyze it */ - dierr = srcImg.OpenImage(pathName, PathProposal::kLocalFssep, true); + CStringA pathNameA(pathName); + dierr = srcImg.OpenImage(pathNameA, PathProposal::kLocalFssep, true); if (dierr != kDIErrNone) { pErrMsg->Format(L"Unable to open disk image: %hs.", DiskImgLib::DIStrError(dierr)); @@ -1625,6 +1628,7 @@ int MainWindow::SSTOpenImage(int seqNum, DiskImg* pDiskImg) int result = -1; CString openFilters, errMsg; CString loadName, saveFolder; + CStringA loadNameA; /* * Select the image to convert. @@ -1650,7 +1654,8 @@ int MainWindow::SSTOpenImage(int seqNum, DiskImg* pDiskImg) fPreferences.SetPrefString(kPrOpenArchiveFolder, saveFolder); /* open the image file and analyze it */ - dierr = pDiskImg->OpenImage(loadName, PathProposal::kLocalFssep, true); + loadNameA = loadName; + dierr = pDiskImg->OpenImage(loadNameA, PathProposal::kLocalFssep, true); if (dierr != kDIErrNone) { errMsg.Format(L"Unable to open disk image: %hs.", DiskImgLib::DIStrError(dierr)); @@ -1930,7 +1935,8 @@ void MainWindow::VolumeCopier(bool openFile) DiskImg::SetAllowWritePhys0(false); - dierr = srcImg.OpenImage(deviceName, '\0', readOnly); + CStringA deviceNameA(deviceName); + dierr = srcImg.OpenImage(deviceNameA, '\0', readOnly); if (dierr == kDIErrAccessDenied) { if (openFile) { errMsg.Format(L"Unable to open '%ls': %hs (try opening the file" diff --git a/app/VolumeCopyDialog.cpp b/app/VolumeCopyDialog.cpp index 8f30dad..3acbddd 100644 --- a/app/VolumeCopyDialog.cpp +++ b/app/VolumeCopyDialog.cpp @@ -618,7 +618,8 @@ void VolumeCopyDialog::OnCopyFromFile(void) CString saveFolder; /* open the image file and analyze it */ - dierr = srcImg.OpenImage(loadName, PathProposal::kLocalFssep, true); + CStringA loadNameA(loadName); + dierr = srcImg.OpenImage(loadNameA, PathProposal::kLocalFssep, true); if (dierr != kDIErrNone) { errMsg.Format(L"Unable to open disk image: %hs.", DiskImgLib::DIStrError(dierr)); diff --git a/diskimg/DiskImg.cpp b/diskimg/DiskImg.cpp index b842126..3b8f2c5 100644 --- a/diskimg/DiskImg.cpp +++ b/diskimg/DiskImg.cpp @@ -381,12 +381,20 @@ bail: return dierr; } +DIError DiskImg::OpenImageFromBufferRO(const uint8_t* buffer, long length) { + return OpenImageFromBuffer(const_cast(buffer), length, true); +} + +DIError DiskImg::OpenImageFromBufferRW(uint8_t* buffer, long length) { + return OpenImageFromBuffer(buffer, length, false); +} + /* * Open from a buffer, which could point to unadorned ready-to-go content * or to a preloaded image file. */ DIError -DiskImg::OpenImage(const void* buffer, long length, bool readOnly) +DiskImg::OpenImageFromBuffer(uint8_t* buffer, long length, bool readOnly) { if (fpDataGFD != NULL) { LOGI(" DI already open!"); @@ -400,8 +408,7 @@ DiskImg::OpenImage(const void* buffer, long length, bool readOnly) fReadOnly = readOnly; pGFDBuffer = new GFDBuffer; - dierr = pGFDBuffer->Open(const_cast(buffer), length, false, false, - readOnly); + dierr = pGFDBuffer->Open(buffer, length, false, false, readOnly); if (dierr != kDIErrNone) { delete pGFDBuffer; return dierr; @@ -782,7 +789,8 @@ DiskImg::AnalyzeImageFile(const char* pathName, char fssep) } else ext = ""; - LOGI(" DI AnalyzeImageFile ext='%s'", ext); + LOGI(" DI AnalyzeImageFile '%s' '%c' ext='%s'", + pathName, fssep, ext); /* sanity check: nobody should have configured these yet */ assert(fOuterFormat == kOuterFormatUnknown); diff --git a/diskimg/DiskImg.h b/diskimg/DiskImg.h index 1d161b0..4939118 100644 --- a/diskimg/DiskImg.h +++ b/diskimg/DiskImg.h @@ -409,7 +409,9 @@ public: // file is on disk; stuff like 2MG headers will be identified and stripped DIError OpenImage(const char* filename, char fssep, bool readOnly); // file is in memory; provide a pointer to the data start and buffer size - DIError OpenImage(const void* buffer, long length, bool readOnly); + DIError OpenImageFromBufferRO(const uint8_t* buffer, long length); + // file is in memory; provide a pointer to the data start and buffer size + DIError OpenImageFromBufferRW(uint8_t* buffer, long length); // file is a range of blocks on an open block-oriented disk image DIError OpenImage(DiskImg* pParent, long firstBlock, long numBlocks); // file is a range of tracks/sectors on an open sector-oriented disk image @@ -741,6 +743,7 @@ private: /* static table of default values */ static const NibbleDescr kStdNibbleDescrs[]; + DIError OpenImageFromBuffer(uint8_t* buffer, long length, bool readOnly); DIError CreateImageCommon(const char* pathName, const char* storageName, bool skipFormat); DIError ValidateCreateFormat(void) const; diff --git a/diskimg/GenericFD.cpp b/diskimg/GenericFD.cpp index 88ed5d0..249740d 100644 --- a/diskimg/GenericFD.cpp +++ b/diskimg/GenericFD.cpp @@ -427,7 +427,7 @@ GFDBuffer::Open(void* buffer, di_off_t length, bool doDelete, bool doExpand, /* if buffer is NULL, allocate it ourselves */ if (buffer == NULL) { - fBuffer = (void*) new char[(int) length]; + fBuffer = (void*) new uint8_t[(int) length]; if (fBuffer == NULL) return kDIErrMalloc; } else