From 082bb032be1f6c75173bf603252e4f37bfded9fa Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 09:45:26 +0200 Subject: [PATCH 01/11] Use mkstemp instead of tmpnam tmpnam isn't threadsafe and shouldn't be used. Fixes compiler warning: ``` warning: the use of 'tmpnam' is dangerous, better use 'mkstemp' ``` --- image/encode/strenc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/image/encode/strenc.c b/image/encode/strenc.c index d6e970e..65c277e 100644 --- a/image/encode/strenc.c +++ b/image/encode/strenc.c @@ -482,8 +482,10 @@ Int StrIOEncInit(CWMImageStrCodec* pSC) pSC->ppTempFile[i] = (char *)malloc(FILENAME_MAX * sizeof(char)); if(pSC->ppTempFile[i] == NULL) return ICERR_ERROR; - if ((pFilename = tmpnam(NULL)) == NULL) + char tmpnambuf[] = {'f', 'i', 'l', 'e', 'X', 'X', 'X', 'X', 'X', 'X', '\0'}; + if (mkstemp(tmpnambuf) == -1) return ICERR_ERROR; + pFilename = tmpnambuf; strcpy(pSC->ppTempFile[i], pFilename); #endif if(CreateWS_File(pSC->ppWStream + i, pFilename, "w+b") != ICERR_OK) return ICERR_ERROR; From 4b02ac18a301408efd93f1608de0837c2c8b7945 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 09:51:51 +0200 Subject: [PATCH 02/11] Remove dead code with dangerously outdated defines The definitions for UINTPTR_T and INTPTR_T are completely broken in this, thankfully unused, header. --- image/sys/image.c | 1 - image/sys/strcodec.h | 1 - image/sys/xplatform_image.h | 84 ------------------------------------- 3 files changed, 86 deletions(-) delete mode 100644 image/sys/xplatform_image.h diff --git a/image/sys/image.c b/image/sys/image.c index c819cbc..8dbb701 100644 --- a/image/sys/image.c +++ b/image/sys/image.c @@ -27,7 +27,6 @@ //*@@@---@@@@****************************************************************** #include "strcodec.h" -// #include "xplatform_image.h" #ifdef MEM_TRACE #define TRACE_MALLOC 1 diff --git a/image/sys/strcodec.h b/image/sys/strcodec.h index 695a454..1e5e6bf 100644 --- a/image/sys/strcodec.h +++ b/image/sys/strcodec.h @@ -31,7 +31,6 @@ #include "windowsmediaphoto.h" #include "common.h" -// #include "xplatform_image.h" // added for Xcode PK universal binary #ifdef __ppc__ diff --git a/image/sys/xplatform_image.h b/image/sys/xplatform_image.h deleted file mode 100644 index cf58230..0000000 --- a/image/sys/xplatform_image.h +++ /dev/null @@ -1,84 +0,0 @@ -//*@@@+++@@@@****************************************************************** -// -// Copyright © Microsoft Corp. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// • Redistributions of source code must retain the above copyright notice, -// this list of conditions and the following disclaimer. -// • Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE -// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE -// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR -// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF -// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS -// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN -// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -// POSSIBILITY OF SUCH DAMAGE. -// -//*@@@---@@@@****************************************************************** - -#ifndef XPLATFORM_IMAGE_H -#define XPLATFORM_IMAGE_H - -#ifdef __ANSI__ -// ANSI -#define FORCE_INLINE -#define CDECL -#define UINTPTR_T unsigned int -#define INTPTR_T int -#define DECLSPEC_ALIGN(bytes) -#endif // __ANSI__ - - -//#if defined(WIN32) -#if defined(WIN32) && !defined(UNDER_CE) // WIN32 seems to be defined always in VS2005 for ARM platform -// x86 -//#define CDECL __cdecl -#define DECLSPEC_ALIGN(bytes) __declspec(align(bytes)) -#endif // x86 - - -#if defined(_ARM_) || defined(UNDER_CE) -// ARM, WinCE -#define FORCE_INLINE inline -#define CDECL -#define UINTPTR_T unsigned int -#define INTPTR_T int -#define DECLSPEC_ALIGN(bytes) - -// parser -#define FULL_PATH_CONFIG_FILE_ENCODE "\\ConfigFile_encode.txt" -#define FULL_PATH_CONFIG_FILE_DECODE "\\ConfigFile_decode.txt" -#define MAX_ARGC 14 -#define MaxCharReadCount 10 -#define MAX_FNAME 256 -#define DELIMITER "filelist:" -#define CODEC_ENCODE "encode" -#define CODEC_DECODE "decode" -#define PHOTON "ptn" -#define OUTRAW "raw" -#define OUTBMP "bmp" -#define OUTPPM "ppm" -#define OUTTIF "tif" -#define OUTHDR "hdr" -#define OUTIYUV "iyuv" -#define OUTYUV422 "yuv422" -#define OUTYUV444 "yuv444" -int XPLATparser(char *pcARGV[], char *pcCodec); -void freeXPLATparser(int iARGC, char *pcARGV[]); - -// WinCE intrinsic -#include -#endif // ARM, WinCE - -#endif // XPLATFORM_IMAGE_H - From bb8ad42864ce752423078c632cae75c1fd4c3c57 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 09:52:56 +0200 Subject: [PATCH 03/11] Fix compile warning about tokens after #endif --- image/sys/strcodec.h | 2 +- jxrtestlib/JXRTestHdr.c | 2 +- jxrtestlib/JXRTestPnm.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/image/sys/strcodec.h b/image/sys/strcodec.h index 1e5e6bf..b1d5e14 100644 --- a/image/sys/strcodec.h +++ b/image/sys/strcodec.h @@ -63,7 +63,7 @@ #ifndef UNREFERENCED_PARAMETER #define UNREFERENCED_PARAMETER(P) { (P) = (P); } -#endif UNREFERENCED_PARAMETER +#endif // UNREFERENCED_PARAMETER #ifdef UNDER_CE #define PLATFORM_WCE diff --git a/jxrtestlib/JXRTestHdr.c b/jxrtestlib/JXRTestHdr.c index a62914c..74697d3 100644 --- a/jxrtestlib/JXRTestHdr.c +++ b/jxrtestlib/JXRTestHdr.c @@ -27,7 +27,7 @@ //*@@@---@@@@****************************************************************** #ifndef ANSI #define _CRT_SECURE_NO_WARNINGS -#endif ANSI +#endif // ANSI #include #include diff --git a/jxrtestlib/JXRTestPnm.c b/jxrtestlib/JXRTestPnm.c index 8917d4a..840c746 100644 --- a/jxrtestlib/JXRTestPnm.c +++ b/jxrtestlib/JXRTestPnm.c @@ -27,7 +27,7 @@ //*@@@---@@@@****************************************************************** #ifndef ANSI #define _CRT_SECURE_NO_WARNINGS -#endif ANSI +#endif // ANSI #include From 38339eee039b844d6ef13b112694aeab3b85282f Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 09:54:03 +0200 Subject: [PATCH 04/11] Remove unused defines This reduces the diff between these two headers and removes useless code. --- image/sys/ansi.h | 9 --------- image/x86/x86.h | 6 ------ 2 files changed, 15 deletions(-) diff --git a/image/sys/ansi.h b/image/sys/ansi.h index 74900c9..07b9566 100644 --- a/image/sys/ansi.h +++ b/image/sys/ansi.h @@ -42,8 +42,6 @@ //================================ // common defines //================================ -#define FORCE_INLINE -#define CDECL #if __LP64__ #define UINTPTR_T unsigned long long #define INTPTR_T long long @@ -52,10 +50,3 @@ #define INTPTR_T int #endif - -//================================ -// quantization optimization -//================================ -//#define RECIP_QUANT_OPT - - diff --git a/image/x86/x86.h b/image/x86/x86.h index c182fc9..76ea196 100644 --- a/image/x86/x86.h +++ b/image/x86/x86.h @@ -50,9 +50,3 @@ #define INTPTR_T intptr_t -//================================ -// quantization optimization -//================================ -#define RECIP_QUANT_OPT - - From 5878421894910417c66b56d5ce7665cb2aa4f048 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 09:55:00 +0200 Subject: [PATCH 05/11] correctly define {U,}INTPTR_T on 64bit Windows Fixes crashes on Windows when loading jxr compressed czi files. This library, despite being originally a Windows product, didn't support 64bit Windows at all. There's a special code path for 32bit Windows with handwritten assembler code, which may have worked back then. But the 64bit code path in the generic "ANSI" platform only checked `__LP64__`, which isn't defined by MSVC. Fix this by using `stdint.h`'s `{u},intptr_t` here to get portable code. --- image/sys/ansi.h | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/image/sys/ansi.h b/image/sys/ansi.h index 07b9566..26a4e7d 100644 --- a/image/sys/ansi.h +++ b/image/sys/ansi.h @@ -27,6 +27,8 @@ //*@@@---@@@@****************************************************************** #pragma once +#include + //================================ // bitio functions //================================ @@ -42,11 +44,5 @@ //================================ // common defines //================================ -#if __LP64__ -#define UINTPTR_T unsigned long long -#define INTPTR_T long long -#else -#define UINTPTR_T unsigned int -#define INTPTR_T int -#endif - +#define UINTPTR_T uintptr_t +#define INTPTR_T intptr_t From ab9c6b78b7ad3205bdb91ef725b09ddbe3c8945d Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 09:56:37 +0200 Subject: [PATCH 06/11] fix warnings about unaligned loads from UBSAN Use memcpy instead to ensure that we don't get warnings about unaligned loads from UBSAN: ``` ../3rdParty/jxrlib/image/decode/segdec.c:66:12: runtime error: load of misaligned address 0x7fc3a0544006 for type 'U32', which requires 4 byte alignment 0x7fc3a0544006: note: pointer points here 01 01 a5 c0 b0 7c 0a 06 05 00 0c 14 10 c2 c0 30 80 38 72 41 ae 1a 8f 54 26 c2 9e f6 c1 25 a9 65 ^ #0 0x7fc3e137429a in _load4 ../3rdParty/jxrlib/image/decode/segdec.c:66 #1 0x7fc3e13748b8 in _flushBit16 ../3rdParty/jxrlib/image/decode/segdec.c:80 #2 0x7fc3e13749a6 in _getBit16 ../3rdParty/jxrlib/image/decode/segdec.c:86 #3 0x7fc3e1385d75 in DecodeMacroblockDC ../3rdParty/jxrlib/image/decode/segdec.c:1224 #4 0x7fc3e131924a in processMacroblockDec ../3rdParty/jxrlib/image/decode/strdec.c:412 #5 0x7fc3e137207a in ImageStrDecDecode ../3rdParty/jxrlib/image/decode/strdec.c:4003 #6 0x7fc3e126c0b2 in PKImageDecode_Copy_WMP ../3rdParty/jxrlib/jxrgluelib/JXRGlueJxr.c:1874 ``` --- image/decode/segdec.c | 4 +++- image/sys/strcodec.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/image/decode/segdec.c b/image/decode/segdec.c index fb83f2b..3c890af 100644 --- a/image/decode/segdec.c +++ b/image/decode/segdec.c @@ -63,7 +63,9 @@ static U32 _FORCEINLINE _load4(void* pv) v |= ((U32)((U16 *) pv)[1]) << 16; return _byteswap_ulong(v); #else // _M_IA64 - return _byteswap_ulong(*(U32*)pv); + U32 v; + memcpy(&v, pv, sizeof(U32)); + return _byteswap_ulong(v); #endif // _M_IA64 #endif // _BIG__ENDIAN_ } diff --git a/image/sys/strcodec.c b/image/sys/strcodec.c index c746d6f..b0989dd 100644 --- a/image/sys/strcodec.c +++ b/image/sys/strcodec.c @@ -694,7 +694,9 @@ U32 load4BE(void* pv) v |= ((U32)((U16 *) pv)[1]) << 16; return _byteswap_ulong(v); #else // _M_IA64 - return _byteswap_ulong(*(U32*)pv); + U32 v; + memcpy(&v, pv, sizeof(U32)); + return _byteswap_ulong(v); #endif // _M_IA64 #endif // _BIG__ENDIAN_ } From a684f95783f2f81bd13bf1f8b03ceb12aa87d661 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 10:00:09 +0200 Subject: [PATCH 07/11] fix undefined behavior for left-shift of -1 My hunch is that (-1 << 31) tries to build INT_MIN, so use that directly. Compare: 1 << 31 = 2147483648 INT_MIN = -2147483648 --- image/sys/adapthuff.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/image/sys/adapthuff.c b/image/sys/adapthuff.c index a690889..cd83034 100644 --- a/image/sys/adapthuff.c +++ b/image/sys/adapthuff.c @@ -28,6 +28,8 @@ #include "strcodec.h" +#include + #ifdef MEM_TRACE #define TRACE_MALLOC 1 #define TRACE_NEW 0 @@ -459,7 +461,7 @@ Void AdaptDiscriminant (CAdaptiveHuffman *pAdHuff) assert (t < gMaxTables[iSym]); //pAdHuff->m_iDiscriminant >>= 1; - pAdHuff->m_iLowerBound = (t == 0) ? (-1 << 31) : -THRESHOLD; + pAdHuff->m_iLowerBound = (t == 0) ? INT_MIN : -THRESHOLD; pAdHuff->m_iUpperBound = (t == gMaxTables[iSym] - 1) ? (1 << 30) : THRESHOLD; switch (iSym) { From 6538ccdd0dcbc69b145057bb6604eb15ea1bec69 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Thu, 8 Dec 2022 15:23:38 -0500 Subject: [PATCH 08/11] Avoid left-shifting negative values to fix UBSAN warning Instead, shift as an unsigned value, then convert back to signed: ``` ../3rdParty/jxrlib/image/decode/segdec.c:1081:36: runtime error: left shift of negative value -1 #0 0x7f0cc5c997c8 in DecodeMacroblockLowpass ../3rdParty/jxrlib/image/decode/segdec.c:1081 #1 0x7f0cc5c2f4f4 in processMacroblockDec ../3rdParty/jxrlib/image/decode/strdec.c:417 #2 0x7f0cc5c881f8 in ImageStrDecDecode ../3rdParty/jxrlib/image/decode/strdec.c:4010 #3 0x7f0cc5b82102 in PKImageDecode_Copy_WMP ../3rdParty/jxrlib/jxrgluelib/JXRGlueJxr.c:1874 ``` Co-authored-by: Milian Wolff --- image/decode/segdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/image/decode/segdec.c b/image/decode/segdec.c index 3c890af..4793d14 100644 --- a/image/decode/segdec.c +++ b/image/decode/segdec.c @@ -961,7 +961,7 @@ Int DecodeMacroblockLowpass (CWMImageStrCodec * pSC, CCodingContext *pContext, pCoeffs[k] += getBits (pIO, iModelBits); } else if (pCoeffs[k] < 0) { - pCoeffs[k] <<= iModelBits; + pCoeffs[k] = (int) (((unsigned) pCoeffs[k]) << iModelBits); pCoeffs[k] -= getBits (pIO, iModelBits); } #endif // WIN32 From f89713da8368dadfaa1c1fd640cb1f6584e69f56 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 10:02:47 +0200 Subject: [PATCH 09/11] Fix compiler warnings with MSVC 2019 --- common/include/guiddef.h | 2 ++ image/sys/strcodec.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/common/include/guiddef.h b/common/include/guiddef.h index 17e0ed3..d3b78cc 100644 --- a/common/include/guiddef.h +++ b/common/include/guiddef.h @@ -90,7 +90,9 @@ typedef struct _GUID { EXTERN_C const GUID FAR name #endif // INITGUID +#if _MSC_VER < 1900 #define DEFINE_OLEGUID(name, l, w1, w2) DEFINE_GUID(name, l, w1, w2, 0xC0,0,0,0,0,0,0,0x46) +#endif #ifndef _GUIDDEF_H_ #define _GUIDDEF_H_ diff --git a/image/sys/strcodec.c b/image/sys/strcodec.c index b0989dd..9b0a7d1 100644 --- a/image/sys/strcodec.c +++ b/image/sys/strcodec.c @@ -671,6 +671,7 @@ ERR detach_SB(SimpleBitIO* pSB) #ifdef _BIG__ENDIAN_ #define _byteswap_ulong(x) (x) #else // _BIG__ENDIAN_ +#if _MSC_VER < 1924 U32 _byteswap_ulong(U32 bits) { U32 r = (bits & 0xffu) << 24; @@ -680,6 +681,7 @@ U32 _byteswap_ulong(U32 bits) return r; } +#endif // _MSC_VER #endif // _BIG__ENDIAN_ #endif From d91810402206c4bdf593d71797ccf119797ce1c3 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 10:30:46 +0200 Subject: [PATCH 10/11] Fix memory leaks when handling OOM scenario If the second or third allocation failed, the code would leak the first and/or secon allocation. Free all buffers if we return early to prevent this. --- jxrtestlib/JXRTestYUV.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/jxrtestlib/JXRTestYUV.c b/jxrtestlib/JXRTestYUV.c index 52dc4ca..b803387 100644 --- a/jxrtestlib/JXRTestYUV.c +++ b/jxrtestlib/JXRTestYUV.c @@ -120,6 +120,9 @@ ERR PKImageEncode_WritePixels_IYUV( if(pY == NULL || pU == NULL || pV == NULL) { + free(pY); + free(pU); + free(pV); return ICERR_ERROR; } @@ -198,6 +201,9 @@ ERR PKImageEncode_WritePixels_YUV422( if(pY == NULL || pU == NULL || pV == NULL) { + free(pY); + free(pU); + free(pV); return ICERR_ERROR; } //YYUV @@ -273,6 +279,9 @@ ERR PKImageEncode_WritePixels_YUV444( if(pY == NULL || pU == NULL || pV == NULL) { + free(pY); + free(pU); + free(pV); return ICERR_ERROR; } @@ -491,6 +500,9 @@ ERR PKImageDecode_Copy_IYUV( if(pY == NULL || pU == NULL || pV == NULL) { + free(pY); + free(pU); + free(pV); return ICERR_ERROR; } @@ -564,6 +576,9 @@ ERR PKImageDecode_Copy_YUV422( if(pY == NULL || pU == NULL || pV == NULL) { + free(pY); + free(pU); + free(pV); return ICERR_ERROR; } @@ -635,6 +650,9 @@ ERR PKImageDecode_Copy_YUV444( if(pY == NULL || pU == NULL || pV == NULL) { + free(pY); + free(pU); + free(pV); return ICERR_ERROR; } From d24f78438da7815cf0025bcc663ae3ad1a0fea38 Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Mon, 7 Jun 2021 10:48:18 +0200 Subject: [PATCH 11/11] Include wchar.h to explicitly define wcslen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes compiler warning: ``` ../jxrgluelib/JXRGlueJxr.c:66:48: warning: implicit declaration of function ‘wcslen’ [-Wimplicit-function-declaration] 66 | U32 uiCBWithNull = sizeof(U16) * ((U32)wcslen((wchar_t *) var.VT.pwszVal) + 1); // +1 for NULL term; | ^~~~~~ ``` --- jxrgluelib/JXRGlueJxr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/jxrgluelib/JXRGlueJxr.c b/jxrgluelib/JXRGlueJxr.c index 1745acf..f60a2e3 100644 --- a/jxrgluelib/JXRGlueJxr.c +++ b/jxrgluelib/JXRGlueJxr.c @@ -27,6 +27,7 @@ // //*@@@---@@@@****************************************************************** #include +#include #include