Skip to content

Cleanups#1

Merged
bgilbert merged 11 commits intomainfrom
cleanup
Dec 8, 2022
Merged

Cleanups#1
bgilbert merged 11 commits intomainfrom
cleanup

Conversation

@bgilbert
Copy link
Copy Markdown
Owner

@bgilbert bgilbert commented Dec 8, 2022

Based on 4creators#2. Drops the added API and CMake support from that PR. Also reworks the signed-left-shift fix to avoid possible problems at INT_MIN.

milianw and others added 11 commits June 7, 2021 09:45
tmpnam isn't threadsafe and shouldn't be used. Fixes compiler warning:
```
warning: the use of 'tmpnam' is dangerous, better use 'mkstemp'
```
The definitions for UINTPTR_T and INTPTR_T are completely broken
in this, thankfully unused, header.
This reduces the diff between these two headers and removes
useless code.
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.
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
```
My hunch is that (-1 << 31) tries to build INT_MIN, so use that
directly. Compare:

1 << 31 = 2147483648
INT_MIN = -2147483648
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 <milian.wolff@kdab.com>
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.
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;
      |                                                ^~~~~~
```
@bgilbert bgilbert merged commit a43dcde into main Dec 8, 2022
@bgilbert bgilbert deleted the cleanup branch December 8, 2022 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants