libE57Format icon indicating copy to clipboard operation
libE57Format copied to clipboard

Possible bug with casting & ternary operators

Open asmaloney opened this issue 4 years ago • 0 comments

I was just looking over some code and ran across this in SourceDestBufferImpl.cpp:

void SourceDestBufferImpl::setNextInt64( int64_t value )
{
...
      case E57_BOOL:
         *reinterpret_cast<bool *>( p ) = ( value ? false : true );
         break;
...

This also happens in:

template <typename T> void SourceDestBufferImpl::_setNextReal( T inValue )

and

void SourceDestBufferImpl::setNextInt64( int64_t value, double scale, double offset )

These ternaries seem backwards... If value is non-zero, I would expect the bool to be true.

I'm surprised we don't see bugs pop up but I suppose it's possible this code never runs.

I'm hesitant to change it without tests in place, but I would appreciate another set of eyes to look at it!

(These could probably just be rewritten using static_cast anyways.)

asmaloney avatar Jul 22 '21 23:07 asmaloney