Skip to content

Commit 7928ec6

Browse files
issakomimalaterre
authored andcommitted
BUG: fixed regression in gdcmImageCodec.cxx
S. https://sourceforge.net/p/gdcm/bugs/554/ An unexpected behavior was introduced in commit 4ecd77a, causing pixel values being all greater or equal to zero on DICOM images where negative values are expected. When trying to solve the following cppcheck issues: shiftNegativeLHS,GDCM/Source/MediaStorageAndFileFormat/gdcmImageCodec.cxx:465,portability,Shifting a negative value is technically undefined behaviour shiftNegativeLHS,GDCM/Source/MediaStorageAndFileFormat/gdcmImageCodec.cxx:521,portability,Shifting a negative value is technically undefined behaviour The changes in gdcmImageCodec.cxx:465 and gdcmImageCodec.cxx:521 implies a potential difference in the sign of nmask, depending on the specific values of PF.GetBitsAllocated() and PF.GetBitsStored(). If the right shift operation in the modified version produces a non-negative value, the result will be positive, whereas the result in the non modified version will always be negative due to the initial assignment of 0x8000 as a signed integer.
1 parent d31fb17 commit 7928ec6

File tree

1 file changed

+4
-2
lines changed

1 file changed

+4
-2
lines changed

Source/MediaStorageAndFileFormat/gdcmImageCodec.cxx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,8 @@ bool ImageCodec::CleanupUnusedBits(char * data8, size_t datalen)
460460
smask = (uint16_t)(
461461
smask << ( 16 - (PF.GetBitsAllocated() - PF.GetBitsStored() + 1) ));
462462
// nmask : to propagate sign bit on negative values
463-
int16_t nmask = (int16_t)(0x8000U >> ( PF.GetBitsAllocated() - PF.GetBitsStored() - 1 ));
463+
int16_t nmask = (int16_t)0x8000;
464+
nmask = (int16_t)(nmask >> ( PF.GetBitsAllocated() - PF.GetBitsStored() - 1 ));
464465

465466
uint16_t *start = (uint16_t*)data;
466467
for( uint16_t *p = start ; p != start + datalen / 2; ++p )
@@ -515,7 +516,8 @@ bool ImageCodec::DoOverlayCleanup(std::istream &is, std::ostream &os)
515516
smask = (uint16_t)(
516517
smask << ( 16 - (PF.GetBitsAllocated() - PF.GetBitsStored() + 1) ));
517518
// nmask : to propagate sign bit on negative values
518-
int16_t nmask = (int16_t)(0x8000U >> ( PF.GetBitsAllocated() - PF.GetBitsStored() - 1 ));
519+
int16_t nmask = (int16_t)0x8000;
520+
nmask = (int16_t)(nmask >> ( PF.GetBitsAllocated() - PF.GetBitsStored() - 1 ));
519521

520522
uint16_t c;
521523
while( is.read((char*)&c,2) )

0 commit comments

Comments
 (0)