From: <sag...@us...> - 2012-03-21 00:36:31
|
Revision: 1226 http://modplug.svn.sourceforge.net/modplug/?rev=1226&view=rev Author: saga-games Date: 2012-03-21 00:36:20 +0000 (Wed, 21 Mar 2012) Log Message: ----------- [Ref] Moved XI / XM instrument / sample handling to XMTools.cpp to avoid duplicate code. Let's see if / when the new code explodes... [Ref] Removed hard to use and hard to understand SpaceToNullStringFixed method and replaced it by easy to understand, easy to use and most importantly safer ReadString / WriteString methods. It's never ever possible to overflow the destination buffer with those, and if source buffer is larger than destination buffer, absolutely no adjustments have to be done (that was previously not the case). [Ref] Removed unused ADPCM packing code (it's not 1997 anymore). Unpacking does still work, though. [Fix] Fixed possible stack corruption when loading instruments into XM files. Modified Paths: -------------- trunk/OpenMPT/common/StringFixer.h trunk/OpenMPT/installer/filetypes.iss trunk/OpenMPT/mptrack/AutoSaver.cpp trunk/OpenMPT/mptrack/Ctrl_ins.cpp trunk/OpenMPT/mptrack/Moddoc.cpp trunk/OpenMPT/mptrack/mptrack_08.vcproj trunk/OpenMPT/mptrack/mptrack_10.vcxproj trunk/OpenMPT/mptrack/mptrack_10.vcxproj.filters trunk/OpenMPT/mptrack/test/test.cpp trunk/OpenMPT/soundlib/ITTools.cpp trunk/OpenMPT/soundlib/ITTools.h trunk/OpenMPT/soundlib/LOAD_AMF.CPP trunk/OpenMPT/soundlib/LOAD_DBM.CPP trunk/OpenMPT/soundlib/LOAD_DMF.CPP trunk/OpenMPT/soundlib/LOAD_DSM.CPP trunk/OpenMPT/soundlib/Load_669.cpp trunk/OpenMPT/soundlib/Load_ams.cpp trunk/OpenMPT/soundlib/Load_far.cpp trunk/OpenMPT/soundlib/Load_gdm.cpp trunk/OpenMPT/soundlib/Load_imf.cpp trunk/OpenMPT/soundlib/Load_it.cpp trunk/OpenMPT/soundlib/Load_itp.cpp trunk/OpenMPT/soundlib/Load_mdl.cpp trunk/OpenMPT/soundlib/Load_med.cpp trunk/OpenMPT/soundlib/Load_mod.cpp trunk/OpenMPT/soundlib/Load_mt2.cpp trunk/OpenMPT/soundlib/Load_mtm.cpp trunk/OpenMPT/soundlib/Load_okt.cpp trunk/OpenMPT/soundlib/Load_psm.cpp trunk/OpenMPT/soundlib/Load_ptm.cpp trunk/OpenMPT/soundlib/Load_s3m.cpp trunk/OpenMPT/soundlib/Load_stm.cpp trunk/OpenMPT/soundlib/Load_ult.cpp trunk/OpenMPT/soundlib/Load_xm.cpp trunk/OpenMPT/soundlib/Sampleio.cpp trunk/OpenMPT/soundlib/Sndfile.cpp trunk/OpenMPT/soundlib/Sndfile.h trunk/OpenMPT/soundlib/load_j2b.cpp Added Paths: ----------- trunk/OpenMPT/soundlib/XMTools.cpp trunk/OpenMPT/soundlib/XMTools.h Removed Paths: ------------- trunk/OpenMPT/unlha/Debug/ trunk/OpenMPT/unrar/Debug/ Modified: trunk/OpenMPT/common/StringFixer.h =================================================================== --- trunk/OpenMPT/common/StringFixer.h 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/common/StringFixer.h 2012-03-21 00:36:20 UTC (rev 1226) @@ -25,40 +25,6 @@ } - // Convert a 0-terminated string to a space-padded string - template <size_t size> - void NullToSpaceString(char (&buffer)[size]) - //------------------------------------------ - { - STATIC_ASSERT(size > 0); - size_t pos = size; - while (pos-- > 0) - if (buffer[pos] == 0) - buffer[pos] = 32; - buffer[size - 1] = 0; - } - - - // Convert a space-padded string to a 0-terminated string - template <size_t size> - void SpaceToNullString(char (&buffer)[size]) - //------------------------------------------ - { - STATIC_ASSERT(size > 0); - // First, remove any Nulls - NullToSpaceString(buffer); - size_t pos = size; - while (pos-- > 0) - { - if (buffer[pos] == 32) - buffer[pos] = 0; - else if(buffer[pos] != 0) - break; - } - buffer[size - 1] = 0; - } - - // Remove any chars after the first null char template <size_t size> void FixNullString(char (&buffer)[size]) @@ -80,41 +46,173 @@ } - // Convert a space-padded string to a 0-terminated string. STATIC VERSION! (use this if the maximum string length is known) - // Additional template parameter to specifify the max length of the final string, - // not including null char (useful for e.g. mod loaders) - template <size_t length, size_t size> - void SpaceToNullStringFixed(char (&buffer)[size]) - //------------------------------------------------ + enum ReadWriteMode { - STATIC_ASSERT(size > 0); - STATIC_ASSERT(length < size); - // Remove Nulls in string - SpaceToNullString(buffer); - // Overwrite trailing chars - for(size_t pos = length; pos < size; pos++) + // Reading / Writing: Standard null-terminated string handling. + nullTerminated, + // Reading: Source string is not guaranteed to be null-terminated (if it fills the whole char array). + // Writing: Destination string is not guaranteed to be null-terminated (if it fills the whole char array). + maybeNullTerminated, + // Reading: String may contain null characters anywhere. They should be treated as spaces. + // Writing: A space-padded string is written. + spacePadded, + // Reading: String may contain null characters anywhere. The last character is ignored (it is supposed to be 0). + // Writing: A space-padded string with a trailing null is written. + spacePaddedNull, + }; + + + // Copy a string from srcBuffer to destBuffer using a given read mode. + // Used for reading strings from files. + // Preferrably use this version of the function, it is safer. + template <ReadWriteMode mode, size_t destSize, size_t srcSize> + void ReadString(char (&destBuffer)[destSize], const char (&srcBuffer)[srcSize]) + //----------------------------------------------------------------------------- + { + STATIC_ASSERT(destSize > 0); + STATIC_ASSERT(srcSize > 0); + ReadString<mode, destSize>(destBuffer, srcBuffer, srcSize); + } + + + // Copy a string from srcBuffer to destBuffer using a given read mode. + // Used for reading strings from files. + // Only use this version of the function if the size of the source buffer is variable. + template <ReadWriteMode mode, size_t destSize> + void ReadString(char (&destBuffer)[destSize], const char *srcBuffer, const size_t srcSize) + //---------------------------------------------------------------------------------------- + { + STATIC_ASSERT(destSize > 0); + ASSERT(srcSize > 0); + + const size_t maxSize = min(destSize, srcSize); + char *dst = destBuffer; + const char *src = srcBuffer; + + if(mode == nullTerminated || mode == maybeNullTerminated) { - buffer[pos] = 0; + // Copy null-terminated string and make sure that destination is null-terminated. + size_t pos = maxSize; + while(pos > 0) + { + pos--; + if((*dst++ = *src++) == '\0') + { + break; + } + } + // Fill rest of string with nulls. + memset(dst, '\0', destSize - maxSize + pos); + + if(mode == nullTerminated) + { + // We assume that the last character of the source buffer is null. + destBuffer[maxSize - 1] = '\0'; + } else + { + // Last character of source buffer may actually be a valid character. + destBuffer[min(destSize - 1, srcSize)] = '\0'; + } + + } else if(mode == spacePadded || mode == spacePaddedNull) + { + // Copy string over, but convert null characters to spaces. + size_t pos = maxSize; + while(pos > 0) + { + *dst = *src; + if(*dst == '\0') + { + *dst = ' '; + } + pos--; + dst++; + src++; + } + // Fill rest of string with nulls. + memset(dst, '\0', destSize - maxSize); + + if(mode == spacePaddedNull && srcSize <= destSize) + { + // We assumed that the last character of the source buffer should be ignored, so make sure it's really null. + destBuffer[srcSize - 1] = '\0'; + } + + // Trim trailing spaces. + pos = maxSize; + dst = destBuffer + pos - 1; + while(pos > 0) + { + if(*dst == ' ') + { + *dst = '\0'; + } else if(*dst != '\0') + { + break; + } + pos--; + dst--; + } + + SetNullTerminator(destBuffer); } } - // Convert a space-padded string to a 0-terminated string. DYNAMIC VERSION! - // Additional function parameter to specifify the max length of the final string, - // not including null char (useful for e.g. mod loaders) - template <size_t size> - void SpaceToNullStringFixed(char (&buffer)[size], size_t length) - //-------------------------------------------------------------- + // Copy a string from srcBuffer to destBuffer using a given write mode. + // Used for writing strings to files. + // Preferrably use this version of the function, it is safer. + template <ReadWriteMode mode, size_t destSize, size_t srcSize> + void WriteString(char (&destBuffer)[destSize], const char (&srcBuffer)[srcSize]) + //------------------------------------------------------------------------------ { - STATIC_ASSERT(size > 0); - ASSERT(length < size); - // Remove Nulls in string - SpaceToNullString(buffer); - // Overwrite trailing chars - for(size_t pos = length; pos < size; pos++) + STATIC_ASSERT(destSize > 0); + STATIC_ASSERT(srcSize > 0); + WriteString<mode, destSize>(destBuffer, srcBuffer, srcSize); + } + + // Copy a string from srcBuffer to destBuffer using a given write mode. + // Used for writing strings to files. + // Only use this version of the function if the size of the source buffer is variable. + template <ReadWriteMode mode, size_t destSize> + void WriteString(char (&destBuffer)[destSize], const char *srcBuffer, const size_t srcSize) + //----------------------------------------------------------------------------------------- + { + STATIC_ASSERT(destSize > 0); + ASSERT(srcSize > 0); + + const size_t maxSize = min(destSize, srcSize); + char *dst = destBuffer; + const char *src = srcBuffer; + + // First, copy over null-terminated string. + size_t pos = maxSize; + while(pos > 0) { - buffer[pos] = 0; + if((*dst = *src) == '\0') + { + break; + } + pos--; + dst++; + src++; } + + if(mode == nullTerminated || mode == maybeNullTerminated) + { + // Fill rest of string with nulls. + memset(dst, '\0', destSize - maxSize + pos); + } else if(mode == spacePadded || mode == spacePaddedNull) + { + // Fill the rest of the destination string with spaces. + memset(dst, ' ', destSize - maxSize + pos); + } + + if(mode == nullTerminated || mode == spacePaddedNull) + { + // Make sure that destination is really null-terminated. + SetNullTerminator(destBuffer); + } } }; Modified: trunk/OpenMPT/installer/filetypes.iss =================================================================== --- trunk/OpenMPT/installer/filetypes.iss 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/installer/filetypes.iss 2012-03-21 00:36:20 UTC (rev 1226) @@ -29,7 +29,7 @@ Name: "associate_exotic\gdm"; Description: "General Digital Music (GDM)"; Name: "associate_exotic\imf"; Description: "Imago Orpheus (IMF)"; Name: "associate_exotic\j2b"; Description: "Jazz Jackrabbit 2 Music (J2B)"; -Name: "associate_exotic\mdl"; Description: "DigiTracker (MDL)"; +Name: "associate_exotic\mdl"; Description: "DigiTrakker (MDL)"; Name: "associate_exotic\med"; Description: "OctaMED (MED)"; Name: "associate_exotic\mo3"; Description: "MO3 compressed modules (MO3)"; Name: "associate_exotic\mt2"; Description: "MadTracker 2 (MT2)"; Modified: trunk/OpenMPT/mptrack/AutoSaver.cpp =================================================================== --- trunk/OpenMPT/mptrack/AutoSaver.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/mptrack/AutoSaver.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -277,22 +277,26 @@ switch (pModDoc->GetModType()) { case MOD_TYPE_MOD: - success = pSndFile->SaveMod(fileName, 0); + success = pSndFile->SaveMod(fileName); break; + case MOD_TYPE_S3M: - success = pSndFile->SaveS3M(fileName, 0); + success = pSndFile->SaveS3M(fileName); break; + case MOD_TYPE_XM: - success = pSndFile->SaveXM(fileName, 0); + success = pSndFile->SaveXM(fileName); break; + case MOD_TYPE_IT: success = (pSndFile->m_dwSongFlags & SONG_ITPROJECT) ? - pSndFile->SaveITProject(fileName) : - pSndFile->SaveIT(fileName, 0); + pSndFile->SaveITProject(fileName) : + pSndFile->SaveIT(fileName); break; + case MOD_TYPE_MPT: //Using IT save function also for MPT. - success = pSndFile->SaveIT(fileName, 0); + success = pSndFile->SaveIT(fileName); break; //default: //Do nothing Modified: trunk/OpenMPT/mptrack/Ctrl_ins.cpp =================================================================== --- trunk/OpenMPT/mptrack/Ctrl_ins.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/mptrack/Ctrl_ins.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -1475,17 +1475,20 @@ _tsplitpath(lpszFileName, nullptr, nullptr, szName, szExt); CMainFrame::GetSettings().SetWorkingDirectory(lpszFileName, DIR_INSTRUMENTS, true); - if (!pIns->name[0]) + if (!pIns->name[0] && m_pSndFile->GetModSpecifications().instrNameLengthMax > 0) { - szName[m_pSndFile->GetModSpecifications().instrNameLengthMax - 1] = 0; - strcpy(pIns->name, szName); + strncpy(pIns->name, szName, CountOf(pIns->name) - 1); + ASSERT(m_pSndFile->GetModSpecifications().instrNameLengthMax < CountOf(pIns->name)); + pIns->name[m_pSndFile->GetModSpecifications().instrNameLengthMax] = '\0'; } - if (!pIns->filename[0]) + if (!pIns->filename[0] && m_pSndFile->GetModSpecifications().instrFilenameLengthMax > 0) { strcat(szName, szExt); - szName[m_pSndFile->GetModSpecifications().instrFilenameLengthMax - 1] = 0; - strcpy(pIns->filename, szName); + strncpy(pIns->filename, szName, CountOf(pIns->filename) - 1); + ASSERT(m_pSndFile->GetModSpecifications().instrFilenameLengthMax < CountOf(pIns->filename)); + pIns->filename[m_pSndFile->GetModSpecifications().instrFilenameLengthMax] = '\0'; } + SetCurrentInstrument(m_nInstrument); if (m_pModDoc) { @@ -1707,7 +1710,7 @@ if (m_pModDoc) { CSoundFile *pSndFile = m_pModDoc->GetSoundFile(); - if ((pSndFile->m_nType & (MOD_TYPE_IT | MOD_TYPE_MPT)) && (pSndFile->m_nInstruments > 0)) + if(pSndFile->m_nInstruments > 0) { BOOL bFirst = (pSndFile->m_nInstruments) ? FALSE : TRUE; LONG ins = m_pModDoc->InsertInstrument(INSTRUMENTINDEX_INVALID, m_nInstrument); @@ -1753,7 +1756,7 @@ else m_nInstrument++; - if(m_nInstrument > m_pSndFile->GetNumInstruments()) + if(m_nInstrument > m_pSndFile->GetNumInstruments()) OnInstrumentNew(); } Modified: trunk/OpenMPT/mptrack/Moddoc.cpp =================================================================== --- trunk/OpenMPT/mptrack/Moddoc.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/mptrack/Moddoc.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -432,7 +432,6 @@ //-------------------------------------------------------------------------- { static int greccount = 0; - UINT dwPacking = 0; BOOL bOk = FALSE; m_SndFile.m_dwLastSavedWithVersion = MptVersion::num; if (!lpszPathName) @@ -444,18 +443,18 @@ greccount++; bOk = DoSave(NULL, TRUE); greccount--; - return bOk; + return bOk; } BeginWaitCursor(); ClearLog(); FixNullStrings(); switch(type) { - case MOD_TYPE_MOD: bOk = m_SndFile.SaveMod(lpszPathName, dwPacking); break; - case MOD_TYPE_S3M: bOk = m_SndFile.SaveS3M(lpszPathName, dwPacking); break; - case MOD_TYPE_XM: bOk = m_SndFile.SaveXM(lpszPathName, dwPacking); break; - case MOD_TYPE_IT: bOk = (m_SndFile.m_dwSongFlags & SONG_ITPROJECT) ? m_SndFile.SaveITProject(lpszPathName) : m_SndFile.SaveIT(lpszPathName, dwPacking); break; - case MOD_TYPE_MPT: bOk = m_SndFile.SaveIT(lpszPathName, dwPacking); break; + case MOD_TYPE_MOD: bOk = m_SndFile.SaveMod(lpszPathName); break; + case MOD_TYPE_S3M: bOk = m_SndFile.SaveS3M(lpszPathName); break; + case MOD_TYPE_XM: bOk = m_SndFile.SaveXM(lpszPathName); break; + case MOD_TYPE_IT: bOk = (m_SndFile.m_dwSongFlags & SONG_ITPROJECT) ? m_SndFile.SaveITProject(lpszPathName) : m_SndFile.SaveIT(lpszPathName); break; + case MOD_TYPE_MPT: bOk = m_SndFile.SaveIT(lpszPathName); break; } EndWaitCursor(); if (bOk) @@ -649,10 +648,6 @@ return TRUE; } else { -// -> CODE#0023 -// -> DESC="IT project files (.itp)" -// ErrorBox(IDS_ERR_SAVESONG, CMainFrame::GetMainFrame()); // done in OnSaveDocument() -// -! NEW_FEATURE#0023 return FALSE; } } @@ -1871,10 +1866,10 @@ switch (type) { case MOD_TYPE_XM: - m_SndFile.SaveXM(files.first_file.c_str(), 0, true); + m_SndFile.SaveXM(files.first_file.c_str(), true); break; case MOD_TYPE_IT: - m_SndFile.SaveIT(files.first_file.c_str(), 0, true); + m_SndFile.SaveIT(files.first_file.c_str(), true); break; } ShowLog(); Modified: trunk/OpenMPT/mptrack/mptrack_08.vcproj =================================================================== --- trunk/OpenMPT/mptrack/mptrack_08.vcproj 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/mptrack/mptrack_08.vcproj 2012-03-21 00:36:20 UTC (rev 1226) @@ -517,6 +517,10 @@ > </File> <File + RelativePath="..\soundlib\XMTools.cpp" + > + </File> + <File RelativePath=".\SampleEditorDialogs.cpp" > </File> @@ -1151,6 +1155,10 @@ > </File> <File + RelativePath=".\soundlib\XMTools.h" + > + </File> + <File RelativePath="..\common\Reporting.h" > </File> Modified: trunk/OpenMPT/mptrack/mptrack_10.vcxproj =================================================================== --- trunk/OpenMPT/mptrack/mptrack_10.vcxproj 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/mptrack/mptrack_10.vcxproj 2012-03-21 00:36:20 UTC (rev 1226) @@ -175,6 +175,7 @@ <ClCompile Include="..\soundlib\ModInstrument.cpp" /> <ClCompile Include="..\soundlib\ModSample.cpp" /> <ClCompile Include="..\soundlib\RowVisitor.cpp" /> + <ClCompile Include="..\soundlib\XMTools.cpp" /> <ClCompile Include="AbstractVstEditor.cpp" /> <ClCompile Include="ACMConvert.cpp" /> <ClCompile Include="ArrayUtils.cpp" /> @@ -339,6 +340,7 @@ <ClInclude Include="..\soundlib\PluginMixBuffer.h" /> <ClInclude Include="..\soundlib\PlugInterface.h" /> <ClInclude Include="..\soundlib\RowVisitor.h" /> + <ClInclude Include="..\soundlib\XMTools.h" /> <ClInclude Include="ACMConvert.h" /> <ClInclude Include="Autotune.h" /> <ClInclude Include="EffectInfo.h" /> Modified: trunk/OpenMPT/mptrack/mptrack_10.vcxproj.filters =================================================================== --- trunk/OpenMPT/mptrack/mptrack_10.vcxproj.filters 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/mptrack/mptrack_10.vcxproj.filters 2012-03-21 00:36:20 UTC (rev 1226) @@ -448,6 +448,9 @@ <ClCompile Include="..\soundlib\ModChannel.cpp"> <Filter>Source Files</Filter> </ClCompile> + <ClCompile Include="..\soundlib\XMTools.cpp"> + <Filter>Module Loaders</Filter> + </ClCompile> </ItemGroup> <ItemGroup> <ClInclude Include="AbstractVstEditor.h"> @@ -795,6 +798,9 @@ <ClInclude Include="..\soundlib\RowVisitor.h"> <Filter>Header Files</Filter> </ClInclude> + <ClInclude Include="..\soundlib\XMTools.h"> + <Filter>Module Loaders</Filter> + </ClInclude> </ItemGroup> <ItemGroup> <None Include="res\bitmap1.bmp"> Modified: trunk/OpenMPT/mptrack/test/test.cpp =================================================================== --- trunk/OpenMPT/mptrack/test/test.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/mptrack/test/test.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -17,6 +17,7 @@ #include "../version.h" #include "../../soundlib/MIDIMacros.h" #include "../../common/misc_util.h" +#include "../../common/StringFixer.h" #include "../serialization_utils.h" #include <limits> #include <fstream> @@ -74,6 +75,7 @@ void TestLoadSaveFile(); void TestPCnoteSerialization(); void TestMisc(); +void TestStringIO(); @@ -83,9 +85,10 @@ { DO_TEST(TestVersion); DO_TEST(TestTypes); - //DO_TEST(TestPCnoteSerialization); + DO_TEST(TestPCnoteSerialization); DO_TEST(TestMisc); - DO_TEST(TestLoadSaveFile) + DO_TEST(TestLoadSaveFile); + DO_TEST(TestStringIO); Log(TEXT("Tests were run\n")); } @@ -855,6 +858,125 @@ } +// Test String I/O functionality +void TestStringIO() +//----------------- +{ + char src1[4] = { 'X', ' ', '\0', 'X' }; // Weird buffer (hello Impulse Tracker) + char src2[4] = { 'X', 'Y', 'Z', ' ' }; // Full buffer, last character space + char src3[4] = { 'X', 'Y', 'Z', '!' }; // Full buffer, last character non-space + char dst1[6]; // Destination buffer, larger than source buffer + char dst2[3]; // Destination buffer, smaller than source buffer + +#define ReadTest(mode, dst, src, expectedResult) \ + StringFixer::ReadString<StringFixer::##mode>(dst, src); \ + VERIFY_EQUAL_NONCONT(strncmp(dst, expectedResult, CountOf(dst)), 0); + +#define WriteTest(mode, dst, src, expectedResult) \ + StringFixer::WriteString<StringFixer::##mode>(dst, src); \ + VERIFY_EQUAL_NONCONT(strncmp(dst, expectedResult, CountOf(dst)), 0); + + // Check reading of null-terminated string into larger buffer + ReadTest(nullTerminated, dst1, src1, "X "); + ReadTest(nullTerminated, dst1, src2, "XYZ"); + ReadTest(nullTerminated, dst1, src3, "XYZ"); + + // Check reading of string that should be null-terminated, but is maybe too long to still hold the null character. + ReadTest(maybeNullTerminated, dst1, src1, "X "); + ReadTest(maybeNullTerminated, dst1, src2, "XYZ "); + ReadTest(maybeNullTerminated, dst1, src3, "XYZ!"); + + // Check reading of space-padded strings with ignored last character + ReadTest(spacePaddedNull, dst1, src1, "X"); + ReadTest(spacePaddedNull, dst1, src2, "XYZ"); + ReadTest(spacePaddedNull, dst1, src3, "XYZ"); + + // Check reading of space-padded strings + ReadTest(spacePadded, dst1, src1, "X X"); + ReadTest(spacePadded, dst1, src2, "XYZ"); + ReadTest(spacePadded, dst1, src3, "XYZ!"); + + /////////////////////////////// + + // Check reading of null-terminated string into smaller buffer + ReadTest(nullTerminated, dst2, src1, "X "); + ReadTest(nullTerminated, dst2, src2, "XY"); + ReadTest(nullTerminated, dst2, src3, "XY"); + + // Check reading of string that should be null-terminated, but is maybe too long to still hold the null character. + ReadTest(maybeNullTerminated, dst2, src1, "X "); + ReadTest(maybeNullTerminated, dst2, src2, "XY"); + ReadTest(maybeNullTerminated, dst2, src3, "XY"); + + // Check reading of space-padded strings with ignored last character + ReadTest(spacePaddedNull, dst2, src1, "X"); + ReadTest(spacePaddedNull, dst2, src2, "XY"); + ReadTest(spacePaddedNull, dst2, src3, "XY"); + + // Check reading of space-padded strings + ReadTest(spacePadded, dst2, src1, "X"); + ReadTest(spacePadded, dst2, src2, "XY"); + ReadTest(spacePadded, dst2, src3, "XY"); + + /////////////////////////////// + + // Check writing of null-terminated string into larger buffer + WriteTest(nullTerminated, dst1, src1, "X "); + WriteTest(nullTerminated, dst1, src2, "XYZ "); + WriteTest(nullTerminated, dst1, src3, "XYZ!"); + + // Check writing of string that should be null-terminated, but is maybe too long to still hold the null character. + WriteTest(maybeNullTerminated, dst1, src1, "X "); + WriteTest(maybeNullTerminated, dst1, src2, "XYZ "); + WriteTest(maybeNullTerminated, dst1, src3, "XYZ!"); + + // Check writing of space-padded strings with last character set to null + WriteTest(spacePaddedNull, dst1, src1, "X "); + WriteTest(spacePaddedNull, dst1, src2, "XYZ "); + WriteTest(spacePaddedNull, dst1, src3, "XYZ! "); + + // Check writing of space-padded strings + WriteTest(spacePadded, dst1, src1, "X "); + WriteTest(spacePadded, dst1, src2, "XYZ "); + WriteTest(spacePadded, dst1, src3, "XYZ! "); + + /////////////////////////////// + + // Check writing of null-terminated string into smaller buffer + WriteTest(nullTerminated, dst2, src1, "X "); + WriteTest(nullTerminated, dst2, src2, "XY"); + WriteTest(nullTerminated, dst2, src3, "XY"); + + // Check writing of string that should be null-terminated, but is maybe too long to still hold the null character. + WriteTest(maybeNullTerminated, dst2, src1, "X "); + WriteTest(maybeNullTerminated, dst2, src2, "XYZ"); + WriteTest(maybeNullTerminated, dst2, src3, "XYZ"); + + // Check writing of space-padded strings with last character set to null + WriteTest(spacePaddedNull, dst2, src1, "X "); + WriteTest(spacePaddedNull, dst2, src2, "XY"); + WriteTest(spacePaddedNull, dst2, src3, "XY"); + + // Check writing of space-padded strings + WriteTest(spacePadded, dst2, src1, "X "); + WriteTest(spacePadded, dst2, src2, "XYZ"); + WriteTest(spacePadded, dst2, src3, "XYZ"); + + /////////////////////////////// + + // Test FixNullString() + StringFixer::FixNullString(src1); + StringFixer::FixNullString(src2); + StringFixer::FixNullString(src3); + VERIFY_EQUAL_NONCONT(strncmp(src1, "X ", CountOf(src1)), 0); + VERIFY_EQUAL_NONCONT(strncmp(src2, "XYZ", CountOf(src2)), 0); + VERIFY_EQUAL_NONCONT(strncmp(src3, "XYZ", CountOf(src3)), 0); + +#undef ReadTest +#undef WriteTest + +} + }; //Namespace MptTest #else //Case: ENABLE_TESTS is not defined. Modified: trunk/OpenMPT/soundlib/ITTools.cpp =================================================================== --- trunk/OpenMPT/soundlib/ITTools.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/ITTools.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -124,10 +124,8 @@ return; } - memcpy(mptIns.name, name, 26); - StringFixer::SpaceToNullStringFixed<25>(mptIns.name); - memcpy(mptIns.filename, filename, 13); - StringFixer::SpaceToNullStringFixed<12>(mptIns.filename); + StringFixer::ReadString<StringFixer::spacePaddedNull>(mptIns.name, name); + StringFixer::ReadString<StringFixer::nullTerminated>(mptIns.filename, filename); // Volume / Panning mptIns.nFadeOut = LittleEndianW(fadeout) << 6; @@ -195,10 +193,8 @@ id = LittleEndian(ITInstrument::magic); trkvers = LittleEndianW(0x0214); - memcpy(filename, mptIns.filename, 13); - StringFixer::FixNullString(filename); - memcpy(name, mptIns.name, 26); - StringFixer::FixNullString(name); + StringFixer::WriteString<StringFixer::nullTerminated>(filename, mptIns.filename); + StringFixer::WriteString<StringFixer::nullTerminated>(name, mptIns.name); // Volume / Panning fadeout = LittleEndianW(static_cast<uint16>(min(mptIns.nFadeOut >> 5, 256))); @@ -278,10 +274,8 @@ return 0; } - memcpy(mptIns.name, name, 26); - StringFixer::SpaceToNullStringFixed<25>(mptIns.name); - memcpy(mptIns.filename, filename, 13); - StringFixer::SpaceToNullStringFixed<12>(mptIns.filename); + StringFixer::ReadString<StringFixer::spacePaddedNull>(mptIns.name, name); + StringFixer::ReadString<StringFixer::nullTerminated>(mptIns.filename, filename); // Volume / Panning mptIns.nFadeOut = LittleEndianW(fadeout) << 5; @@ -438,10 +432,8 @@ // Header id = LittleEndian(ITSample::magic); - memcpy(filename, mptSmp.filename, 13); - StringFixer::FixNullString(filename); - //memcpy(name, m_szNames[nsmp], 26); - //StringFixer::FixNullString(name); + StringFixer::WriteString<StringFixer::nullTerminated>(filename, mptSmp.filename); + //StringFixer::WriteString<StringFixer::nullTerminated>(name, m_szNames[nsmp]); // Volume / Panning gvl = static_cast<BYTE>(mptSmp.nGlobalVol); @@ -507,8 +499,7 @@ return 0; } - memcpy(mptSmp.filename, filename, 13); - StringFixer::SpaceToNullStringFixed<12>(mptSmp.filename); + StringFixer::ReadString<StringFixer::nullTerminated>(mptSmp.filename, filename); mptSmp.uFlags = 0; Modified: trunk/OpenMPT/soundlib/ITTools.h =================================================================== --- trunk/OpenMPT/soundlib/ITTools.h 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/ITTools.h 2012-03-21 00:36:20 UTC (rev 1226) @@ -47,7 +47,7 @@ }; uint32 id; // Magic Bytes (IMPM) - char songname[26]; // Song Name (duh) + char songname[26]; // Song Name, null-terminated (but may also contain nulls) uint8 highlight_minor; // Rows per Beat highlight uint8 highlight_major; // Rows per Measure highlight uint16 ordnum; // Number of Orders @@ -65,7 +65,7 @@ uint8 sep; // Pan Separation (0...128) uint8 pwd; // Pitch Wheel Depth uint16 msglength; // Length of Song Message - uint32 msgoffset; // Offset of Song Message in File + uint32 msgoffset; // Offset of Song Message in File (IT crops message after first null) uint32 reserved; // ChibiTracker writes "CHBI" here. OpenMPT writes "OMPT" here in some cases, see Load_it.cpp uint8 chnpan[64]; // Initial Channel Panning uint8 chnvol[64]; // Initial Channel Volume @@ -124,7 +124,7 @@ }; uint32 id; // Magic Bytes (IMPI) - char filename[13]; // DOS Filename + char filename[13]; // DOS Filename, null-terminated uint8 flags; // Volume Envelope Flags uint8 vls; // Envelope Loop Start uint8 vle; // Envelope Loop End @@ -137,7 +137,7 @@ uint16 trkvers; // Tracker ID uint8 nos; // Number of embedded samples char reserved2; // Reserved - char name[26]; // Instrument Name + char name[26]; // Instrument Name, null-terminated (but may also contain nulls) char reserved3[6]; // Even more reserved bytes uint8 keyboard[240]; // Sample / Transpose map uint8 volenv[200]; // This appears to be a pre-computed (interpolated) version of the volume envelope data found below. @@ -167,7 +167,7 @@ }; uint32 id; // Magic Bytes (IMPI) - char filename[13]; // DOS Filename + char filename[13]; // DOS Filename, null-terminated uint8 nna; // New Note Action uint8 dct; // Duplicate Note Check Type uint8 dca; // Duplicate Note Check Action @@ -181,7 +181,7 @@ uint16 trkvers; // Tracker ID uint8 nos; // Number of embedded samples char reserved1; // Reserved - char name[26]; // Instrument Name + char name[26]; // Instrument Name, null-terminated (but may also contain nulls) uint8 ifc; // Filter Cutoff uint8 ifr; // Filter Resonance uint8 mch; // MIDI Channel @@ -246,15 +246,15 @@ cvtSignedSample = 0x01, cvtIT215Compression = 0x04, - cvtADPCMSample = 0xFF + cvtADPCMSample = 0xFF, // MODPlugin :( }; uint32 id; // Magic Bytes (IMPS) - char filename[13]; // DOS Filename + char filename[13]; // DOS Filename, null-terminated uint8 gvl; // Global Volume uint8 flags; // Sample Flags uint8 vol; // Default Volume - char name[26]; // Sample Name + char name[26]; // Sample Name, null-terminated (but may also contain nulls) uint8 cvt; // Sample Import Format uint8 dfp; // Sample Panning uint32 length; // Sample Length (in samples) Modified: trunk/OpenMPT/soundlib/LOAD_AMF.CPP =================================================================== --- trunk/OpenMPT/soundlib/LOAD_AMF.CPP 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/LOAD_AMF.CPP 2012-03-21 00:36:20 UTC (rev 1226) @@ -193,8 +193,8 @@ for (UINT iSmp=0; iSmp<numsamples; iSmp++) { ModSample *psmp = &Samples[iSmp+1]; - memcpy(m_szNames[iSmp + 1], lpStream+dwMemPos, 22); - StringFixer::SpaceToNullStringFixed<22>(m_szNames[iSmp + 1]); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[iSmp + 1], reinterpret_cast<const char *>(lpStream + dwMemPos), 22); + psmp->nGlobalVol = 64; psmp->nFineTune = MOD2XMFineTune(lpStream[dwMemPos+22]); psmp->nVolume = lpStream[dwMemPos+23]; @@ -267,8 +267,9 @@ || (!pfh->numsamples) || (pfh->numsamples > MAX_SAMPLES) || (pfh->numchannels < 1) || (pfh->numchannels > 32)) return false; - memcpy(m_szNames[0], pfh->title, 31); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[0]); + + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], pfh->title); + dwMemPos = sizeof(AMFFILEHEADER); m_nType = MOD_TYPE_AMF; m_nChannels = pfh->numchannels; @@ -327,10 +328,9 @@ AMFSAMPLE *psh = (AMFSAMPLE *)(lpStream + dwMemPos); dwMemPos += sizeof(AMFSAMPLE); - memcpy(m_szNames[iIns+1], psh->samplename, 31); - memcpy(pSmp->filename, psh->filename, 13); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[iIns + 1]); - StringFixer::SpaceToNullStringFixed<13>(pSmp->filename); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[iIns + 1], psh->samplename); + StringFixer::ReadString<StringFixer::nullTerminated>(pSmp->filename, psh->filename); + pSmp->nLength = LittleEndian(psh->length); pSmp->nC5Speed = LittleEndianW(psh->c2spd); pSmp->nGlobalVol = 64; Modified: trunk/OpenMPT/soundlib/LOAD_DBM.CPP =================================================================== --- trunk/OpenMPT/soundlib/LOAD_DBM.CPP 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/LOAD_DBM.CPP 2012-03-21 00:36:20 UTC (rev 1226) @@ -194,8 +194,16 @@ nPatterns = BigEndianW(pfh->patterns); m_nType = MOD_TYPE_DBM; m_nChannels = CLAMP(BigEndianW(pfh->channels), 1, MAX_BASECHANNELS); // note: MAX_BASECHANNELS is currently 127, but DBM supports up to 128 channels. - memcpy(m_szNames[0], (pfh->songname[0]) ? pfh->songname : pfh->songname2, 32); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[0]); + + if(pfh->songname[0]) + { + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], pfh->songname); + } else + { + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], pfh->songname2); + + } + Order.resize(nOrders, Order.GetInvalidPatIndex()); for (UINT iOrd=0; iOrd < nOrders; iOrd++) { @@ -240,12 +248,10 @@ Instruments[iIns + 1] = pIns; - memcpy(pIns->name, pih->name, 30); - StringFixer::SpaceToNullStringFixed<30>(pIns->name); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(pIns->name, pih->name); if (psmp) { - memcpy(m_szNames[nsmp], pih->name, 30); - StringFixer::SpaceToNullStringFixed<30>(m_szNames[nsmp]); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[nsmp], pih->name); } pIns->nFadeOut = 1024; // ??? Modified: trunk/OpenMPT/soundlib/LOAD_DMF.CPP =================================================================== --- trunk/OpenMPT/soundlib/LOAD_DMF.CPP 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/LOAD_DMF.CPP 2012-03-21 00:36:20 UTC (rev 1226) @@ -749,11 +749,8 @@ ASSERT_CAN_READ_SAMPLE(1); const size_t lenName = lpStream[dwMemPos++]; - STATIC_ASSERT(MAX_SAMPLENAME > 30); - const size_t lenNameImport = min(30, lenName); ASSERT_CAN_READ_SAMPLE(lenName); - memcpy(pSndFile->m_szNames[nSmp], lpStream + dwMemPos, lenNameImport); - StringFixer::SpaceToNullStringFixed(pSndFile->m_szNames[nSmp], lenNameImport); + StringFixer::ReadString<StringFixer::spacePadded>(pSndFile->m_szNames[nSmp], reinterpret_cast<const char *>(lpStream + dwMemPos), lenName); dwMemPos += lenName; ASSERT_CAN_READ_SAMPLE(sizeof(DMFCHUNK_SAMPLEHEADER)); @@ -797,8 +794,7 @@ { // Read library name in version 8 files ASSERT_CAN_READ_SAMPLE(8); - memcpy(sample.filename, lpStream + dwMemPos, 8); - StringFixer::SpaceToNullStringFixed<8>(sample.filename); + StringFixer::ReadString<StringFixer::spacePadded>(sample.filename, reinterpret_cast<const char *>(lpStream + dwMemPos), 8); dwMemPos += 8; } @@ -828,8 +824,7 @@ } dwMemPos += sizeof(DMFHEADER); - memcpy(m_szNames[0], pHeader->songname, 30); - StringFixer::SpaceToNullStringFixed<30>(m_szNames[0]); + StringFixer::ReadString<StringFixer::spacePadded>(m_szNames[0], pHeader->songname); m_nChannels = 0; #ifdef MODPLUG_TRACKER Modified: trunk/OpenMPT/soundlib/LOAD_DSM.CPP =================================================================== --- trunk/OpenMPT/soundlib/LOAD_DSM.CPP 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/LOAD_DSM.CPP 2012-03-21 00:36:20 UTC (rev 1226) @@ -120,8 +120,9 @@ ChnSettings[iPan].nPan = psong->panpos[iPan] << 1; } } - memcpy(m_szNames[0], psong->songname, 28); - StringFixer::SpaceToNullStringFixed<28>(m_szNames[0]); + + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], psong->songname); + nPat = 0; nSmp = 1; while (dwMemPos < dwMemLength - 8) @@ -214,11 +215,12 @@ if (dwMemPos + pSmp->inst_len >= dwMemLength - 8) break; DWORD dwPos = dwMemPos + sizeof(DSMSAMPLE); dwMemPos += 8 + pSmp->inst_len; - memcpy(m_szNames[nSmp], pSmp->samplename, 28); - StringFixer::SpaceToNullStringFixed<28>(m_szNames[nSmp]); + ModSample *psmp = &Samples[nSmp]; - memcpy(psmp->filename, pSmp->filename, 13); - StringFixer::SpaceToNullStringFixed<13>(psmp->filename); + + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[nSmp], pSmp->samplename); + StringFixer::ReadString<StringFixer::nullTerminated>(psmp->filename, pSmp->filename); + psmp->nGlobalVol = 64; psmp->nC5Speed = pSmp->c2spd; psmp->uFlags = (WORD)((pSmp->flags & 1) ? CHN_LOOP : 0); Modified: trunk/OpenMPT/soundlib/Load_669.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_669.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_669.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -30,10 +30,10 @@ typedef struct tagSAMPLE669 { - BYTE filename[13]; - BYTE length[4]; // when will somebody think about DWORD align ??? - BYTE loopstart[4]; - BYTE loopend[4]; + char filename[13]; + uint32 length; // when will somebody think about DWORD align ??? + uint32 loopStart; + uint32 loopEnd; } SAMPLE669; @@ -54,7 +54,7 @@ if (dontfuckwithme > dwMemLength) return false; for (UINT ichk=0; ichk<pfh->samples; ichk++) { - DWORD len = LittleEndian(*((DWORD *)(&psmp[ichk].length))); + DWORD len = LittleEndian(psmp[ichk].length); dontfuckwithme += len; } if (dontfuckwithme - 0x1F1 > dwMemLength) return false; @@ -67,15 +67,15 @@ m_nDefaultSpeed = 6; m_nChannels = 8; - memcpy(m_szNames[0], pfh->songmessage, min(MAX_SAMPLENAME - 1, 36)); - StringFixer::SpaceToNullStringFixed<min(MAX_SAMPLENAME - 1, 36)>(m_szNames[0]); + // Copy first song message line into song title + StringFixer::ReadString<StringFixer::spacePadded>(m_szNames[0], reinterpret_cast<const char *>(pfh->songmessage), 36); m_nSamples = pfh->samples; for (SAMPLEINDEX nSmp = 1; nSmp <= m_nSamples; nSmp++, psmp++) { - DWORD len = LittleEndian(*((DWORD *)(&psmp->length))); - DWORD loopstart = LittleEndian(*((DWORD *)(&psmp->loopstart))); - DWORD loopend = LittleEndian(*((DWORD *)(&psmp->loopend))); + DWORD len = LittleEndian(psmp->length); + DWORD loopstart = LittleEndian(psmp->loopStart); + DWORD loopend = LittleEndian(psmp->loopEnd); if (len > MAX_SAMPLE_LENGTH) len = MAX_SAMPLE_LENGTH; if ((loopend > len) && (!loopstart)) loopend = 0; if (loopend > len) loopend = len; @@ -84,8 +84,7 @@ Samples[nSmp].nLoopStart = loopstart; Samples[nSmp].nLoopEnd = loopend; if (loopend) Samples[nSmp].uFlags |= CHN_LOOP; - memcpy(m_szNames[nSmp], psmp->filename, 13); - StringFixer::SpaceToNullStringFixed<13>(m_szNames[nSmp]); + StringFixer::ReadString<StringFixer::nullTerminated>(m_szNames[nSmp], psmp->filename); Samples[nSmp].nVolume = 256; Samples[nSmp].nGlobalVol = 64; Samples[nSmp].nPan = 128; Modified: trunk/OpenMPT/soundlib/Load_ams.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_ams.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_ams.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -112,34 +112,28 @@ // Read Song Name if (dwMemPos + 1 >= dwMemLength) return true; tmp = lpStream[dwMemPos++]; - if (dwMemPos + tmp + 1 >= dwMemLength) return true; - tmp2 = (tmp < 32) ? tmp : 31; - if (tmp2) memcpy(m_szNames[0], lpStream + dwMemPos, tmp2); - StringFixer::SpaceToNullStringFixed(m_szNames[0], tmp2); - m_szNames[0][tmp2] = 0; + if (dwMemPos + tmp >= dwMemLength) return true; + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], reinterpret_cast<const char *>(lpStream + dwMemPos), tmp); + dwMemPos += tmp; // Read sample names for (UINT sNam=1; sNam<=m_nSamples; sNam++) { - if (dwMemPos + 32 >= dwMemLength) return true; + if (dwMemPos + 1 >= dwMemLength) return true; tmp = lpStream[dwMemPos++]; - tmp2 = (tmp < 32) ? tmp : 31; - if (tmp2) memcpy(m_szNames[sNam], lpStream+dwMemPos, tmp2); - StringFixer::SpaceToNullStringFixed(m_szNames[sNam], tmp2); + if (dwMemPos + tmp >= dwMemLength) return true; + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[sNam], reinterpret_cast<const char *>(lpStream + dwMemPos), tmp); dwMemPos += tmp; } // Read Channel names for (UINT cNam=0; cNam<m_nChannels; cNam++) { - if (dwMemPos + 32 >= dwMemLength) return true; - BYTE chnnamlen = lpStream[dwMemPos++]; - if ((chnnamlen) && (chnnamlen < MAX_CHANNELNAME)) - { - memcpy(ChnSettings[cNam].szName, lpStream + dwMemPos, chnnamlen); - StringFixer::SpaceToNullStringFixed(ChnSettings[cNam].szName, chnnamlen); - } + if (dwMemPos + 1 >= dwMemLength) return true; + uint8 chnnamlen = lpStream[dwMemPos++]; + if (dwMemPos + tmp >= dwMemLength) return true; + StringFixer::ReadString<StringFixer::maybeNullTerminated>(ChnSettings[cNam].szName, reinterpret_cast<const char *>(lpStream + dwMemPos), chnnamlen); dwMemPos += chnnamlen; } Modified: trunk/OpenMPT/soundlib/Load_far.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_far.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_far.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -21,8 +21,9 @@ typedef struct FARHEADER1 { DWORD id; // file magic FAR= - CHAR songname[40]; // songname - CHAR magic2[3]; // 13,10,26 + char songname[32]; // songname + uint8 reserved[8]; + char eofMagic[3]; // 13,10,26 WORD headerlen; // remaining length of header in bytes BYTE version; // 0xD1 BYTE onoff[16]; @@ -71,8 +72,12 @@ UINT headerlen; BYTE samplemap[8]; - if ((!lpStream) || (dwMemLength < 1024) || (LittleEndian(pmh1->id) != FARFILEMAGIC) - || (pmh1->magic2[0] != 13) || (pmh1->magic2[1] != 10) || (pmh1->magic2[2] != 26)) return false; + if ((!lpStream) || (dwMemLength < 1024) || (pmh1->id != LittleEndian(FARFILEMAGIC)) + || (pmh1->eofMagic[0] != 13) || (pmh1->eofMagic[1] != 10) || (pmh1->eofMagic[2] != 26)) + { + return false; + } + headerlen = LittleEndianW(pmh1->headerlen); pmh1->stlen = LittleEndianW( pmh1->stlen ); /* inplace byteswap -- Toad */ if ((headerlen >= dwMemLength) || (dwMemPos + pmh1->stlen + sizeof(FARHEADER2) >= dwMemLength)) return false; @@ -86,8 +91,7 @@ m_nDefaultTempo = 80; m_nDefaultGlobalVolume = MAX_GLOBAL_VOLUME; - memcpy(m_szNames[0], pmh1->songname, 31); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[0]); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], pmh1->songname); // Channel Setting for (UINT nchpan=0; nchpan<16; nchpan++) @@ -244,9 +248,10 @@ const FARSAMPLE *pfs = reinterpret_cast<const FARSAMPLE*>(lpStream + dwMemPos); dwMemPos += sizeof(FARSAMPLE); m_nSamples = ismp + 1; - memcpy(m_szNames[ismp+1], pfs->samplename, 31); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[ismp + 1]); - const DWORD length = LittleEndian( pfs->length ); + + StringFixer::ReadString<StringFixer::nullTerminated>(m_szNames[ismp+1], pfs->samplename); + + const DWORD length = LittleEndian(pfs->length); pSmp->nLength = length; pSmp->nLoopStart = LittleEndian(pfs->reppos) ; pSmp->nLoopEnd = LittleEndian(pfs->repend) ; Modified: trunk/OpenMPT/soundlib/Load_gdm.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_gdm.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_gdm.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -105,8 +105,7 @@ // Song name MemsetZero(m_szNames); - memcpy(m_szNames[0], pHeader->SongTitle, 32); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[0]); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], pHeader->SongTitle); // Read channel pan map... 0...15 = channel panning, 16 = surround channel, 255 = channel does not exist m_nChannels = 32; @@ -166,10 +165,8 @@ // Sample header - memcpy(m_szNames[iSmp], pSample->SamName, 32); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[iSmp]); - memcpy(Samples[iSmp].filename, pSample->FileName, 12); - StringFixer::SpaceToNullStringFixed<12>(Samples[iSmp].filename); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[iSmp], pSample->SamName); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(Samples[iSmp].filename, pSample->FileName); Samples[iSmp].nC5Speed = LittleEndianW(pSample->C4Hertz); Samples[iSmp].nGlobalVol = 256; // not supported in this format Modified: trunk/OpenMPT/soundlib/Load_imf.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_imf.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_imf.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -293,8 +293,7 @@ // song name MemsetZero(m_szNames); - memcpy(m_szNames[0], hdr.title, 31); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[0]); + StringFixer::ReadString<StringFixer::nullTerminated>(m_szNames[0], hdr.title); if(hdr.flags & 1) m_dwSongFlags |= SONG_LINEARSLIDES; @@ -312,8 +311,7 @@ ChnSettings[nChn].nPan = hdr.channels[nChn].panning * 64 / 255; ChnSettings[nChn].nPan *= 4; - memcpy(ChnSettings[nChn].szName, hdr.channels[nChn].name, 12); - StringFixer::SpaceToNullStringFixed<12>(ChnSettings[nChn].szName); + StringFixer::ReadString<StringFixer::nullTerminated>(ChnSettings[nChn].szName, hdr.channels[nChn].name); // TODO: reverb/chorus? switch(hdr.channels[nChn].status) @@ -504,8 +502,7 @@ Instruments[nIns + 1] = pIns; - memcpy(pIns->name, imfins.name, 31); - StringFixer::SpaceToNullStringFixed<31>(pIns->name); + StringFixer::ReadString<StringFixer::nullTerminated>(pIns->name, imfins.name); if(imfins.smpnum) { @@ -542,8 +539,7 @@ ModSample &sample = Samples[firstsample + nSmp]; - memcpy(sample.filename, imfsmp.filename, 12); - StringFixer::SpaceToNullStringFixed<12>(sample.filename); + StringFixer::ReadString<StringFixer::nullTerminated>(sample.filename, imfsmp.filename); strcpy(m_szNames[m_nSamples], sample.filename); uint32 byteLen = sample.nLength = LittleEndian(imfsmp.length); Modified: trunk/OpenMPT/soundlib/Load_it.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_it.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_it.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -406,8 +406,7 @@ if ((itHeader.flags & ITFileHeader::reqEmbeddedMIDIConfig) || (itHeader.special & ITFileHeader::embedMIDIConfiguration)) m_dwSongFlags |= SONG_EMBEDMIDICFG; if (itHeader.flags & ITFileHeader::extendedFilterRange) m_dwSongFlags |= SONG_EXFILTERRANGE; - memcpy(m_szNames[0], itHeader.songname, 26); - StringFixer::SpaceToNullStringFixed<26>(m_szNames[0]); + StringFixer::ReadString<StringFixer::spacePaddedNull>(m_szNames[0], itHeader.songname); // Global Volume m_nDefaultGlobalVolume = itHeader.globalvol << 1; @@ -763,8 +762,7 @@ { size_t sampleOffset = pis->ConvertToMPT(Samples[nsmp + 1]); - memcpy(m_szNames[nsmp + 1], pis->name, 26); - StringFixer::SpaceToNullStringFixed<25>(m_szNames[nsmp + 1]); + StringFixer::ReadString<StringFixer::spacePaddedNull>(m_szNames[nsmp + 1], pis->name); lastSampleOffset = Util::Max(lastSampleOffset, sampleOffset + ReadSample(&Samples[nsmp + 1], pis->GetSampleFormat(itHeader.cwtv), (LPSTR)(lpStream + sampleOffset), dwMemLength - sampleOffset)); } @@ -1090,10 +1088,10 @@ #pragma warning(disable:4100) -bool CSoundFile::SaveIT(LPCSTR lpszFileName, UINT nPacking, const bool compatExport) -//---------------------------------------------------------------------------------- +bool CSoundFile::SaveIT(LPCSTR lpszFileName, bool compatibilityExport) +//-------------------------------------------------------------------- { - const CModSpecifications &specs = (GetType() == MOD_TYPE_MPT ? ModSpecs::mptm : (compatExport ? ModSpecs::it : ModSpecs::itEx)); + const CModSpecifications &specs = (GetType() == MOD_TYPE_MPT ? ModSpecs::mptm : (compatibilityExport ? ModSpecs::it : ModSpecs::itEx)); DWORD dwChnNamLen; ITFileHeader itHeader; @@ -1106,7 +1104,7 @@ MemsetZero(itHeader); dwChnNamLen = 0; itHeader.id = ITFileHeader::itMagic; - lstrcpyn(itHeader.songname, m_szNames[0], 26); + StringFixer::WriteString<StringFixer::nullTerminated>(itHeader.songname, m_szNames[0]); itHeader.highlight_minor = (BYTE)min(m_nDefaultRowsPerBeat, 0xFF); itHeader.highlight_major = (BYTE)min(m_nDefaultRowsPerMeasure, 0xFF); @@ -1156,7 +1154,7 @@ } } - if(!compatExport) + if(!compatibilityExport) { // This way, we indicate that the file will most likely contain OpenMPT hacks. Compatibility export puts 0 here. itHeader.reserved = ITFileHeader::omptMagic; @@ -1169,7 +1167,7 @@ if (m_dwSongFlags & SONG_LINEARSLIDES) itHeader.flags |= ITFileHeader::linearSlides; if (m_dwSongFlags & SONG_ITOLDEFFECTS) itHeader.flags |= ITFileHeader::itOldEffects; if (m_dwSongFlags & SONG_ITCOMPATGXX) itHeader.flags |= ITFileHeader::itCompatGxx; - if ((m_dwSongFlags & SONG_EXFILTERRANGE) && !compatExport) itHeader.flags |= ITFileHeader::extendedFilterRange; + if ((m_dwSongFlags & SONG_EXFILTERRANGE) && !compatibilityExport) itHeader.flags |= ITFileHeader::extendedFilterRange; itHeader.globalvol = m_nDefaultGlobalVolume >> 1; itHeader.mv = min(m_nSamplePreAmp, 128); itHeader.speed = m_nDefaultSpeed; @@ -1189,7 +1187,7 @@ } // Channel names - if(!compatExport) + if(!compatibilityExport) { for (UINT ich=0; ich<m_nChannels; ich++) { @@ -1209,14 +1207,14 @@ } // Pattern Names - const PATTERNINDEX numNamedPats = compatExport ? 0 : Patterns.GetNumNamedPatterns(); + const PATTERNINDEX numNamedPats = compatibilityExport ? 0 : Patterns.GetNumNamedPatterns(); if(numNamedPats > 0) { dwExtra += (numNamedPats * MAX_PATTERNNAME) + 8; } // Mix Plugins. Just calculate the size of this extra block for now. - if(!compatExport) + if(!compatibilityExport) { dwExtra += SaveMixPlugins(NULL, TRUE); } @@ -1270,7 +1268,7 @@ } // Writing channel names - if(dwChnNamLen && !compatExport) + if(dwChnNamLen && !compatibilityExport) { DWORD d = LittleEndian(magicChannelNames); // "CNAM" fwrite(&d, 1, 4, f); @@ -1283,7 +1281,7 @@ } // Writing mix plugins info - if(!compatExport) + if(!compatibilityExport) { SaveMixPlugins(f, FALSE); } @@ -1304,12 +1302,12 @@ if(Instruments[nins]) { - instSize = iti.ConvertToIT(*Instruments[nins], compatExport, *this); + instSize = iti.ConvertToIT(*Instruments[nins], compatibilityExport, *this); } else // Save Empty Instrument { ModInstrument dummy; - instSize = iti.ConvertToIT(dummy, compatExport, *this); + instSize = iti.ConvertToIT(dummy, compatibilityExport, *this); } // Writing instrument @@ -1318,7 +1316,7 @@ fwrite(&iti, 1, instSize, f); //------------ rewbs.modularInstData - if (Instruments[nins] && !compatExport) + if (Instruments[nins] && !compatibilityExport) { dwPos += SaveModularInstrumentData(f, Instruments[nins]); } @@ -1417,7 +1415,7 @@ case VOLCMD_TONEPORTAMENTO: vol = 193 + ConvertVolParam(m); break; case VOLCMD_PORTADOWN: vol = 105 + ConvertVolParam(m); break; case VOLCMD_PORTAUP: vol = 115 + ConvertVolParam(m); break; - case VOLCMD_OFFSET: if(!compatExport) vol = 223 + ConvertVolParam(m); //rewbs.volOff + case VOLCMD_OFFSET: if(!compatibilityExport) vol = 223 + ConvertVolParam(m); //rewbs.volOff break; default: vol = 0xFF; } @@ -1425,7 +1423,7 @@ if (vol != 0xFF) b |= 4; if (command) { - S3MSaveConvert(&command, ¶m, true, compatExport); + S3MSaveConvert(&command, ¶m, true, compatibilityExport); if (command) b |= 8; } // Packing information @@ -1532,8 +1530,7 @@ { itss.ConvertToIT(Samples[nsmp], GetType()); - memcpy(itss.name, m_szNames[nsmp], 26); - StringFixer::FixNullString(itss.name); + StringFixer::WriteString<StringFixer::nullTerminated>(itss.name, m_szNames[nsmp]); itss.samplepointer = dwPos; fseek(f, smppos[nsmp - 1], SEEK_SET); @@ -1546,7 +1543,7 @@ } //Save hacked-on extra info - if(!compatExport) + if(!compatibilityExport) { SaveExtendedInstrumentProperties(itHeader.insnum, f); SaveExtendedSongProperties(f); Modified: trunk/OpenMPT/soundlib/Load_itp.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_itp.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_itp.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -336,8 +336,7 @@ { pis.ConvertToMPT(Samples[nsmp]); - memcpy(m_szNames[nsmp], pis.name, 26); - StringFixer::SpaceToNullStringFixed<25>(m_szNames[nsmp]); + StringFixer::ReadString<StringFixer::nullTerminated>(m_szNames[nsmp], pis.name); // Read sample data ReadSample(&Samples[nsmp], pis.GetSampleFormat(), (LPSTR)(lpStream + dwMemPos), len); @@ -622,8 +621,7 @@ ITSample itss; itss.ConvertToIT(Samples[nsmp], GetType()); - memcpy(itss.name, m_szNames[nsmp], 26); - StringFixer::FixNullString(itss.name); + StringFixer::WriteString<StringFixer::nullTerminated>(itss.name, m_szNames[nsmp]); id = nsmp; fwrite(&id, 1, sizeof(id), f); Modified: trunk/OpenMPT/soundlib/Load_mdl.cpp =================================================================== --- trunk/OpenMPT/soundlib/Load_mdl.cpp 2012-03-20 16:27:14 UTC (rev 1225) +++ trunk/OpenMPT/soundlib/Load_mdl.cpp 2012-03-21 00:36:20 UTC (rev 1226) @@ -1,7 +1,7 @@ /* * Load_mdl.cpp * ------------ - * Purpose: DigiTracker (MDL) module loader + * Purpose: DigiTrakker (MDL) module loader * Notes : (currently none) * Authors: Olivier Lapicque * OpenMPT Devs @@ -16,36 +16,77 @@ #pragma warning(disable:4244) //"conversion from 'type1' to 'type2', possible loss of data" -typedef struct MDLSONGHEADER +#pragma pack(push, 1) + +struct MDLFileHeader { DWORD id; // "DMDL" = 0x4C444D44 BYTE version; -} MDLSONGHEADER; +}; -typedef struct MDLINFOBLOCK +struct MDLInfoBlock { - CHAR songname[32]; - CHAR composer[20]; - WORD norders; - WORD repeatpos; - BYTE globalvol; - BYTE speed; - BYTE tempo; - BYTE channelinfo[32]; - BYTE seq[256]; -} MDLINFOBLOCK; + char songname[32]; + char composer[20]; + uint16 norders; + uint16 repeatpos; + uint8 globalvol; + uint8 speed; + uint8 tempo; + uint8 channelinfo[32]; + uint8 seq[256]; +}; -typedef struct MDLPATTERNDATA +struct MDLPatternHeader { - BYTE channels; - BYTE lastrow; // nrows = lastrow+1 - CHAR name[16]; - WORD data[1]; -} MDLPATTERNDATA; + uint8 channels; + uint8 lastrow; // nrows = lastrow+1 + char name[16]; + uint16 data[1]; +}; +struct MDLSampleHeaderCommon +{ + uint8 sampleIndex; + char name[32]; + char filename[8]; +}; + + +struct MDLSampleHeader +{ + MDLSampleHeaderCommon info; + uint32 c4Speed; + uint32 length; + uint32 loopStart; + uint32 loopLength; + uint8 unused; // was volume in v0.0, why it was changed I have no idea + uint8 flags; +}; + +STATIC_ASSERT(sizeof(MDLSampleHeader) == 59); + + +struct MDLSampleHeaderv0 +{ + MDLSampleHeaderCommon info; + uint16 c4Speed; + uint32 length; + uint32 loopStart; + uint32 loopLength; + uint8 volume; + uint8 flags; +}; + +STATIC_ASSERT(sizeof(MDLSampleHeaderv0) == 57); + + +#pragma pack(pop) + + void ConvertMDLCommand(ModCommand *m, UINT eff, UINT data) //-------------------------------------------------------- { @@ -235,8 +276,8 @@ //--------------------------------------------------------------------- { DWORD dwMemPos, dwPos, blocklen, dwTrackPos; - const MDLSONGHEADER *pmsh = (const MDLSONGHEADER *)lpStream; - MDLINFOBLOCK *pmib; + const MDLFileHeader *pmsh = (const MDLFileHeader *)lpStream; + MDLInfoBlock *pmib; UINT i,j, norders = 0, npatterns = 0, ntracks = 0; UINT ninstruments = 0, nsamples = 0; WORD block; @@ -253,10 +294,10 @@ #ifdef MDL_LOG Log("MDL v%d.%d\n", pmsh->version>>4, pmsh->version&0x0f); #endif - memset(patterntracks, 0, sizeof(patterntracks)); - memset(smpinfo, 0, sizeof(smpinfo)); - memset(insvolenv, 0, sizeof(insvolenv)); - memset(inspanenv, 0, sizeof(inspanenv)); + MemsetZero(patterntracks); + MemsetZero(smpinfo); + MemsetZero(insvolenv); + MemsetZero(inspanenv); dwMemPos = 5; dwTrackPos = 0; pvolenv = ppanenv = ppitchenv = NULL; @@ -279,9 +320,8 @@ #ifdef MDL_LOG Log("infoblock: %d bytes\n", blocklen); #endif - pmib = (MDLINFOBLOCK *)(lpStream+dwMemPos); - memcpy(m_szNames[0], pmib->songname, 31); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[0]); + pmib = (MDLInfoBlock *)(lpStream+dwMemPos); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[0], pmib->songname); norders = pmib->norders; if (norders > MAX_ORDERS) norders = MAX_ORDERS; @@ -330,7 +370,7 @@ if (dwPos+18 >= dwMemLength) break; if (pmsh->version > 0) { - const MDLPATTERNDATA *pmpd = (const MDLPATTERNDATA *)(lpStream + dwPos); + const MDLPatternHeader *pmpd = (const MDLPatternHeader *)(lpStream + dwPos); if (pmpd->channels > 32) break; patternLength[i] = pmpd->lastrow + 1; if (m_nChannels < pmpd->channels) m_nChannels = pmpd->channels; @@ -383,9 +423,10 @@ break; } ModInstrument *pIns = Instruments[nins]; - memcpy(pIns->name, lpStream+dwPos+2, 32); - StringFixer::SpaceToNullStringFixed<31>(pIns->name); + // I give up. better rewrite this crap (or take SchismTracker's MDL loader). + StringFixer::ReadString<StringFixer::maybeNullTerminated>(pIns->name, reinterpret_cast<const char *>(lpStream + dwPos + 2), 32); + for (j=0; j<lpStream[dwPos+1]; j++) { const BYTE *ps = lpStream+dwPos+34+14*j; @@ -467,41 +508,77 @@ Log("sample infoblock: %d bytes\n", blocklen); #endif nsamples = lpStream[dwMemPos]; - dwPos = dwMemPos+1; - for (i=0; i<nsamples; i++, dwPos += (pmsh->version > 0) ? 59 : 57) + dwPos = dwMemPos + 1; + for (i = 0; i < nsamples; i++, dwPos += (pmsh->version > 0) ? sizeof(MDLSampleHeader) : sizeof(MDLSampleHeaderv0)) { - UINT nins = lpStream[dwPos]; - if ((nins >= MAX_SAMPLES) || (!nins)) continue; - if (m_nSamples < nins) m_nSamples = nins; - ModSample *pSmp = &Samples[nins]; - memcpy(m_szNames[nins], lpStream+dwPos+1, 31); - memcpy(pSmp->filename, lpStream+dwPos+33, 8); - StringFixer::SpaceToNullStringFixed<31>(m_szNames[nins]); - StringFixer::SpaceToNullStringFixed<8>(pSmp->filename); - const BYTE *p = lpStream+dwPos+41; - if (pmsh->version > 0) + const MDLSampleHeaderCommon *info = reinterpret_cast<const MDLSampleHeaderCommon *>(lpStream + dwPos); + if(info->sampleIndex >= MAX_SAMPLES || info->sampleIndex == 0) { - pSmp->nC5Speed = *((DWORD *)p); - p += 4; + continue; + } + + ModSample &sample = Samples[info->sampleIndex]; + + StringFixer::ReadString<StringFixer::maybeNullTerminated>(m_szNames[info->sampleIndex], info->name); + StringFixer::ReadString<StringFixer::maybeNullTerminated>(sample.filename, info->filename); + + if(pmsh->version > 0) + { + const MDLSampleHeader *sampleHeader = reinterpret_cast<const MDLSampleHeader *>(lpStream + dwPos); + + sample.nC5Speed = LittleEndian(sampleHeader->c4Speed); + sample.nLength = LittleEndian(sampleHeader->length); + sample.nLoopStart = LittleEndian(sampleHeader->loopStart); + sample.nLoopEnd = sample.nLoopStart + LittleEndian(sampleHeader->loopLength); + if(sample.nLoopEnd > sample.nLoopStart) + { + sample.uFlags |= CHN_LOOP; + if((sampleHeader->flags & 0x02)) + { + sample.uFlags |= CHN_PINGPONGLOOP; + } + } + sample.nGlobalVol = 64; + sample.nVolume = 256; + + if((sampleHeader->flags & 0x01)) + { + sample.uFlags |= CHN_16BIT; + sample.nLength /= 2; + sample.nLoopStart /= 2; + sample.nLoopEnd /= 2; + } + + smpinfo[info->sampleIndex] = (sampleHeader->flags >> 2) & 3; } else { - pSmp->nC5Speed = *((WORD *)p); - p += 2; + const MDLSampleHeaderv0 *sampleHeader = reinterpret_cast<const MDLSampleHeaderv0 *>(lpStream + dwPos); + + sample.nC5Speed = LittleEndianW(sampleHeader->c4Speed); + sample.... [truncated message content] |