Re: [Libreoffice] Bug 39428 (https://bugs.freedesktop.org/show_bug.cgi?id=39428#c1)

On 01/06/2012 06:08 PM, Keith McRae wrote:
> Building tools/ results in errors from tools/inc/fract.o,
> tools/inc/tools/gen.o, tools/source/generic/fract.o and
> tools/source/generic/poly.o due to "undefined reference to
> SvStream::operator>>(long&)"
>
> Question: Should I modify these classes to use the sal_ types?

You can either use a temporary sal_Int32 variable for just the operator 
>> call and leave the surrounding code using long, or you can go the 
extra mile of making the surrounding code use sal_Int32, too.  But note 
that this can quickly grow into a large additional task that you would 
probably not want to mix with the task at hand.  So, in all but trivial 
cases it might be best to go with the first option, potentially adding a 
comment to why such a temporary variable is needed (which can be removed 
again when the surrounding code is eventually cleaned up).

> In basic/source/sbx/sbxvalue.cxx - functions SbxValue::LoadData and
> SbxValue::StoreData which both have comments and workarounds for
> SvStream having operator>>/<< (long) defined.

Those workarounds for SbxSALINT64 can be removed then, directly 
operating on the nInt64 member instead.

> In basic/source/runtime/methods1.cxx - function:
>
> sal_Bool lcl_WriteSbxVariable( const SbxVariable& rVar, SvStream* pStrm,
> sal_Bool bBinary, short nBlockLen, sal_Bool bIsArray )
>
>
>          case SbxSALINT64:
>          case SbxSALUINT64:
>                  if( bIsVariant )
>                      *pStrm << (sal_uInt16)SbxSALINT64; // VarType Id
>                  *pStrm << (sal_uInt64)rVar.GetInt64();
>                  break;
>
> BUT, in the corresponding read function:
>
> sal_Bool lcl_ReadSbxVariable( SbxVariable& rVar, SvStream* pStrm,
> sal_Bool bBinary, short nBlockLen, sal_Bool bIsArray )
>
>          case SbxSALINT64:
>          case SbxSALUINT64:
>                  {
>                  sal_uInt32 aInt;
>                  *pStrm >> aInt;
>                  rVar.PutInt64( (sal_Int64)aInt );
>                  }
>                  break;
>
> So it serializes a sal_uInt64 but deserializes a sal_uInt32. Could this
> be a potential problem?

This indeed looks like a bug.

> In editeng/source/rtf/rtfgrf.cxx - function:
>
> static void WriteBMPHeader( SvStream& rStream, const SvxRTFPictureType&
> rPicType )
>
> This makes heavy use of the SWAP_LONG macro from solar.h via an inline
> SwapLong function while serializing sal_ types. SvStream already swaps
> the bytes depending on the endianness, so is this (possible) double
> swapping necessary?

This also does not look right. --- Unless the BMP shall be in platform 
byte-order, in which case the additional swap on big-endian would cancel 
out with SvStream's internal swap.  But 
<http://en.wikipedia.org/wiki/BMP_file_format> states that BMP uses 
little-endian, so this is probably indeed a bug (and exporting RTF 
bitmaps has never been tested with Solaris SPARC OOo...).

Thanks a lot for this in-depth work.  Let us know if you find any other 
obstacles.

Stephan