From 833dbee1673172222e2cc1526f64efed3f003822 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sun, 11 Mar 2018 14:35:36 +0100 Subject: [PATCH] - fixed some issues with m_swap's design. The native byte order converters were defined as macros which hid some issues due to lack of type checks. Additionally the ???Long variants taking 'long' variables were removed, because longs are not always 32 bits so this could be destructive. As a result of this, removed several DWORDs from struct definitions in i_crash.cpp. --- src/m_png.cpp | 2 +- src/m_swap.h | 78 +++++++++++-------- src/resourcefiles/file_zip.cpp | 41 +++++----- .../music_wavewriter_mididevice.cpp | 16 ++-- src/win32/i_crash.cpp | 53 ++++++------- 5 files changed, 104 insertions(+), 86 deletions(-) diff --git a/src/m_png.cpp b/src/m_png.cpp index ee73e0270..23b7e57ce 100644 --- a/src/m_png.cpp +++ b/src/m_png.cpp @@ -264,7 +264,7 @@ bool M_AppendPNGText (FileWriter *file, const char *keyword, const char *text) { crc = AddCRC32 (crc, (uint8_t *)text, len); } - crc = BigLong((unsigned int)crc); + crc = BigLong(crc); return file->Write (&crc, 4) == 4; } return false; diff --git a/src/m_swap.h b/src/m_swap.h index a68a0837b..a3e03bea2 100644 --- a/src/m_swap.h +++ b/src/m_swap.h @@ -46,16 +46,6 @@ inline unsigned int LittleLong(unsigned int x) return OSSwapLittleToHostInt32(x); } -inline int LittleLong(long x) -{ - return OSSwapLittleToHostInt32((uint32_t)x); -} - -inline unsigned int LittleLong(unsigned long x) -{ - return OSSwapLittleToHostInt32((uint32_t)x); -} - inline short BigShort(short x) { return (short)OSSwapBigToHostInt16((uint16_t)x); @@ -76,8 +66,7 @@ inline unsigned int BigLong(unsigned int x) return OSSwapBigToHostInt32(x); } -#else -#ifdef __BIG_ENDIAN__ +#elif defined __BIG_ENDIAN__ // Swap 16bit, that is, MSB and LSB byte. // No masking with 0xFF should be necessary. @@ -120,42 +109,66 @@ inline int LittleLong (int x) | (((unsigned int)x)<<24)); } -inline unsigned int LittleLong(unsigned long x) +inline short BigShort(short x) { - return LittleLong((unsigned int)x); + return x; } -inline int LittleLong(long x) +inline unsigned short BigShort(unsigned short x) { - return LittleLong((int)x); + return x; } -#define BigShort(x) (x) -#define BigLong(x) (x) +inline unsigned int BigLong(unsigned int &x) +{ + return x; +} + +inline int BigLong(int &x) +{ + return x; +} #else -#define LittleShort(x) (x) -#define LittleLong(x) (x) +inline short LittleShort(short x) +{ + return x; +} -#if defined(_MSC_VER) +inline unsigned short LittleShort(unsigned short x) +{ + return x; +} -inline short BigShort (short x) +inline unsigned int LittleLong(unsigned int x) +{ + return x; +} + +inline int LittleLong(int x) +{ + return x; +} + +#ifdef _MSC_VER + +inline short BigShort(short x) { return (short)_byteswap_ushort((unsigned short)x); } -inline unsigned short BigShort (unsigned short x) +inline unsigned short BigShort(unsigned short x) { return _byteswap_ushort(x); } -inline int BigLong (int x) +inline int BigLong(int x) { return (int)_byteswap_ulong((unsigned long)x); } -inline unsigned int BigLong (unsigned int x) +inline unsigned int BigLong(unsigned int x) { return (unsigned int)_byteswap_ulong((unsigned long)x); } @@ -190,10 +203,16 @@ inline int BigLong (int x) | ((((unsigned int)x)<<8) & 0xff0000) | (((unsigned int)x)<<24)); } + #endif #endif // __BIG_ENDIAN__ -#endif // __APPLE__ + +// These may be destructive so they should create errors +unsigned long BigLong(unsigned long) = delete; +long BigLong(long) = delete; +unsigned long LittleLong(unsigned long) = delete; +long LittleLong(long) = delete; // Data accessors, since some data is highly likely to be unaligned. @@ -206,10 +225,6 @@ inline int GetInt(const unsigned char *foo) { return *(const int *)foo; } -inline int GetBigInt(const unsigned char *foo) -{ - return BigLong(GetInt(foo)); -} #else inline int GetShort(const unsigned char *foo) { @@ -219,11 +234,12 @@ inline int GetInt(const unsigned char *foo) { return int(foo[0] | (foo[1] << 8) | (foo[2] << 16) | (foo[3] << 24)); } +#endif inline int GetBigInt(const unsigned char *foo) { return int((foo[0] << 24) | (foo[1] << 16) | (foo[2] << 8) | foo[3]); } -#endif + #ifdef __BIG_ENDIAN__ inline int GetNativeInt(const unsigned char *foo) { diff --git a/src/resourcefiles/file_zip.cpp b/src/resourcefiles/file_zip.cpp index 684a3424f..9bce04a52 100644 --- a/src/resourcefiles/file_zip.cpp +++ b/src/resourcefiles/file_zip.cpp @@ -507,17 +507,19 @@ FResourceFile *CheckZip(const char *filename, FileRdr &file, bool quiet) // //========================================================================== -static void time_to_dos(struct tm *time, unsigned short *dosdate, unsigned short *dostime) +static std::pair time_to_dos(struct tm *time) { + std::pair val; if (time == NULL || time->tm_year < 80) { - *dosdate = *dostime = 0; + val.first = val.second = 0; } else { - *dosdate = LittleShort((time->tm_year - 80) * 512 + (time->tm_mon + 1) * 32 + time->tm_mday); - *dostime = LittleShort(time->tm_hour * 2048 + time->tm_min * 32 + time->tm_sec / 2); + val.first = (time->tm_year - 80) * 512 + (time->tm_mon + 1) * 32 + time->tm_mday; + val.second= time->tm_hour * 2048 + time->tm_min * 32 + time->tm_sec / 2; } + return val; } //========================================================================== @@ -532,7 +534,7 @@ static void time_to_dos(struct tm *time, unsigned short *dosdate, unsigned short // //========================================================================== -int AppendToZip(FileWriter *zip_file, const char *filename, FCompressedBuffer &content, uint16_t date, uint16_t time) +int AppendToZip(FileWriter *zip_file, const char *filename, FCompressedBuffer &content, std::pair &dostime) { FZipLocalFileHeader local; int position; @@ -540,10 +542,10 @@ int AppendToZip(FileWriter *zip_file, const char *filename, FCompressedBuffer &c local.Magic = ZIP_LOCALFILE; local.VersionToExtract[0] = 20; local.VersionToExtract[1] = 0; - local.Flags = content.mMethod == METHOD_DEFLATE ? LittleShort(2) : LittleShort((uint16_t)content.mZipFlags); - local.Method = LittleShort(content.mMethod); - local.ModDate = date; - local.ModTime = time; + local.Flags = content.mMethod == METHOD_DEFLATE ? LittleShort((uint16_t)2) : LittleShort((uint16_t)content.mZipFlags); + local.Method = LittleShort((uint16_t)content.mMethod); + local.ModDate = LittleShort(dostime.first); + local.ModTime = LittleShort(dostime.second); local.CRC32 = content.mCRC32; local.UncompressedSize = LittleLong(content.mSize); local.CompressedSize = LittleLong(content.mCompressedSize); @@ -573,7 +575,7 @@ int AppendToZip(FileWriter *zip_file, const char *filename, FCompressedBuffer &c // //========================================================================== -int AppendCentralDirectory(FileWriter *zip_file, const char *filename, FCompressedBuffer &content, uint16_t date, uint16_t time, int position) +int AppendCentralDirectory(FileWriter *zip_file, const char *filename, FCompressedBuffer &content, std::pair &dostime, int position) { FZipCentralDirectoryInfo dir; @@ -582,10 +584,10 @@ int AppendCentralDirectory(FileWriter *zip_file, const char *filename, FCompress dir.VersionMadeBy[1] = 0; dir.VersionToExtract[0] = 20; dir.VersionToExtract[1] = 0; - dir.Flags = content.mMethod == METHOD_DEFLATE ? LittleShort(2) : LittleShort((uint16_t)content.mZipFlags); - dir.Method = LittleShort(content.mMethod); - dir.ModTime = time; - dir.ModDate = date; + dir.Flags = content.mMethod == METHOD_DEFLATE ? LittleShort((uint16_t)2) : LittleShort((uint16_t)content.mZipFlags); + dir.Method = LittleShort((uint16_t)content.mMethod); + dir.ModTime = LittleShort(dostime.first); + dir.ModDate = LittleShort(dostime.second); dir.CRC32 = content.mCRC32; dir.CompressedSize = LittleLong(content.mCompressedSize); dir.UncompressedSize = LittleLong(content.mSize); @@ -610,10 +612,9 @@ bool WriteZip(const char *filename, TArray &filenames, TArray positions; @@ -624,7 +625,7 @@ bool WriteZip(const char *filename, TArray &filenames, TArray &filenames, TArrayTell(); for (unsigned i = 0; i < filenames.Size(); i++) { - if (AppendCentralDirectory(f, filenames[i], content[i], mydate, mytime, positions[i]) < 0) + if (AppendCentralDirectory(f, filenames[i], content[i], dostime, positions[i]) < 0) { delete f; remove(filename); @@ -650,9 +651,9 @@ bool WriteZip(const char *filename, TArray &filenames, TArrayTell() - dirofs); + dirend.DirectorySize = LittleLong((uint32_t)(f->Tell() - dirofs)); dirend.ZipCommentLength = 0; if (f->Write(&dirend, sizeof(dirend)) != sizeof(dirend)) { diff --git a/src/sound/mididevices/music_wavewriter_mididevice.cpp b/src/sound/mididevices/music_wavewriter_mididevice.cpp index 99590ee88..6bf585636 100644 --- a/src/sound/mididevices/music_wavewriter_mididevice.cpp +++ b/src/sound/mididevices/music_wavewriter_mididevice.cpp @@ -106,18 +106,18 @@ MIDIWaveWriter::MIDIWaveWriter(const char *filename, SoftSynthMIDIDevice *playde playDevice->CalcTickRate(); fmt.ChunkID = MAKE_ID('f','m','t',' '); fmt.ChunkLen = LittleLong(uint32_t(sizeof(fmt) - 8)); - fmt.FormatTag = LittleShort(0xFFFE); // WAVE_FORMAT_EXTENSIBLE - fmt.Channels = LittleShort(2); + fmt.FormatTag = LittleShort((uint16_t)0xFFFE); // WAVE_FORMAT_EXTENSIBLE + fmt.Channels = LittleShort((uint16_t)2); fmt.SamplesPerSec = LittleLong(SampleRate); fmt.AvgBytesPerSec = LittleLong(SampleRate * 8); - fmt.BlockAlign = LittleShort(8); - fmt.BitsPerSample = LittleShort(32); - fmt.ExtensionSize = LittleShort(2 + 4 + 16); - fmt.ValidBitsPerSample = LittleShort(32); + fmt.BlockAlign = LittleShort((uint16_t)8); + fmt.BitsPerSample = LittleShort((uint16_t)32); + fmt.ExtensionSize = LittleShort((uint16_t)(2 + 4 + 16)); + fmt.ValidBitsPerSample = LittleShort((uint16_t)32); fmt.ChannelMask = LittleLong(3); fmt.SubFormatA = LittleLong(0x00000003); // Set subformat to KSDATAFORMAT_SUBTYPE_IEEE_FLOAT - fmt.SubFormatB = LittleShort(0x0000); - fmt.SubFormatC = LittleShort(0x0010); + fmt.SubFormatB = 0x0000; + fmt.SubFormatC = LittleShort((uint16_t)0x0010); fmt.SubFormatD[0] = 0x80; fmt.SubFormatD[1] = 0x00; fmt.SubFormatD[2] = 0x00; diff --git a/src/win32/i_crash.cpp b/src/win32/i_crash.cpp index 2e904cc0f..4caa954e1 100644 --- a/src/win32/i_crash.cpp +++ b/src/win32/i_crash.cpp @@ -166,49 +166,49 @@ namespace zip #pragma pack(push,1) struct LocalFileHeader { - DWORD Magic; // 0 + uint32_t Magic; // 0 uint8_t VersionToExtract[2]; // 4 uint16_t Flags; // 6 uint16_t Method; // 8 uint16_t ModTime; // 10 uint16_t ModDate; // 12 - DWORD CRC32; // 14 - DWORD CompressedSize; // 18 - DWORD UncompressedSize; // 22 + uint32_t CRC32; // 14 + uint32_t CompressedSize; // 18 + uint32_t UncompressedSize; // 22 uint16_t NameLength; // 26 uint16_t ExtraLength; // 28 }; struct CentralDirectoryEntry { - DWORD Magic; - uint8_t VersionMadeBy[2]; - uint8_t VersionToExtract[2]; + uint32_t Magic; + uint8_t VersionMadeBy[2]; + uint8_t VersionToExtract[2]; uint16_t Flags; uint16_t Method; uint16_t ModTime; uint16_t ModDate; - DWORD CRC32; - DWORD CompressedSize; - DWORD UncompressedSize; + uint32_t CRC32; + uint32_t CompressedSize; + uint32_t UncompressedSize; uint16_t NameLength; uint16_t ExtraLength; uint16_t CommentLength; uint16_t StartingDiskNumber; uint16_t InternalAttributes; - DWORD ExternalAttributes; - DWORD LocalHeaderOffset; + uint32_t ExternalAttributes; + uint32_t LocalHeaderOffset; }; struct EndOfCentralDirectory { - DWORD Magic; + uint32_t Magic; uint16_t DiskNumber; uint16_t FirstDisk; uint16_t NumEntries; uint16_t NumEntriesOnAllDisks; - DWORD DirectorySize; - DWORD DirectoryOffset; + uint32_t DirectorySize; + uint32_t DirectoryOffset; uint16_t ZipCommentLength; }; #pragma pack(pop) @@ -219,9 +219,9 @@ struct TarFile HANDLE File; const char *Filename; int ZipOffset; - DWORD UncompressedSize; - DWORD CompressedSize; - DWORD CRC32; + uint32_t UncompressedSize; + uint32_t CompressedSize; + uint32_t CRC32; bool Deflated; }; @@ -1513,7 +1513,8 @@ static HANDLE MakeZip () struct tm *nowtm; int i, numfiles; HANDLE file; - DWORD len, dirsize; + DWORD len; + uint32_t dirsize; size_t namelen; if (NumFiles == 0) @@ -1553,7 +1554,7 @@ static HANDLE MakeZip () central.ModTime = dostime; central.ModDate = dosdate; - dirend.DirectoryOffset = LittleLong(SetFilePointer (file, 0, NULL, FILE_CURRENT)); + dirend.DirectoryOffset = LittleLong((uint32_t)SetFilePointer (file, 0, NULL, FILE_CURRENT)); for (i = 0, numfiles = 0, dirsize = 0; i < NumFiles; ++i) { @@ -1565,8 +1566,8 @@ static HANDLE MakeZip () numfiles++; if (TarFiles[i].Deflated) { - central.Flags = LittleShort(2); - central.Method = LittleShort(8); + central.Flags = LittleShort((uint16_t)2); + central.Method = LittleShort((uint16_t)8); } else { @@ -1577,7 +1578,7 @@ static HANDLE MakeZip () central.InternalAttributes = 0; if (namelen > 4 && stricmp(TarFiles[i].Filename - 4, ".txt") == 0) { // Bit 0 set indicates this is probably a text file. But do any tools use it? - central.InternalAttributes = LittleShort(1); + central.InternalAttributes = LittleShort((uint16_t)1); } central.CRC32 = LittleLong(TarFiles[i].CRC32); central.CompressedSize = LittleLong(TarFiles[i].CompressedSize); @@ -1590,7 +1591,7 @@ static HANDLE MakeZip () } // Write the directory terminator - dirend.NumEntriesOnAllDisks = dirend.NumEntries = LittleShort(numfiles); + dirend.NumEntriesOnAllDisks = dirend.NumEntries = LittleShort((uint16_t)numfiles); dirend.DirectorySize = LittleLong(dirsize); WriteFile (file, &dirend, sizeof(dirend), &len, NULL); @@ -1635,7 +1636,7 @@ static void AddZipFile (HANDLE ziphandle, TarFile *whichfile, short dosdate, sho if (gzip) { - local.Method = LittleShort(8); + local.Method = LittleShort((uint16_t)8); whichfile->Deflated = true; stream.next_out = outbuf; stream.avail_out = sizeof(outbuf); @@ -1643,7 +1644,7 @@ static void AddZipFile (HANDLE ziphandle, TarFile *whichfile, short dosdate, sho // Write out the header and filename. local.VersionToExtract[0] = 20; - local.Flags = gzip ? LittleShort(2) : 0; + local.Flags = gzip ? LittleShort((uint16_t)2) : 0; local.ModTime = dostime; local.ModDate = dosdate; local.UncompressedSize = LittleLong(whichfile->UncompressedSize);