[Qgis-developer] WKB support is only little-endian?

[David Adler]

I looked into this and tried to fix it although it still didn't work.

The basic problem is in QgsGeometryFactory::geomFromWkb
which has the logic:

  //find out type (bytes 2-5)
   int type;
   memcpy( &type, wkb + 1, sizeof( int ) );
   QgsAbstractGeometryV2* geom = nullptr;
   geom = geomFromWkbType( QgsWKBTypes::Type( type ) );

which is not checking the endian-ness before copying the 4 bytes 
containing the wkb type
QgsConstWkbPtr::readHeader() appears to have the correct logic to read 
and return the type and seems like a better approach anyway.

I changed QgsGeometryFactory to

   QgsConstWkbPtr wkbPtr( wkb );
   QgsWKBTypes::Type type = wkbPtr.readHeader();
   QgsAbstractGeometryV2* geom = nullptr;
   geom = geomFromWkbType( type );

with this change, it worked for same-endian but still failed for 
different-endian.

I believe there is also a defect in QgsConstWkbPtr::readHeader()
It correctly sets mEndianSwap.
Then it gets the type with
( *this ) >> wkbType;

The logic seems to make sense in that it then tests mEndianSwap and if 
set, calls endian_swap to swap the bytes in wkbType.
The problem is that the >> operator in
( *this ) >> wkbType
already takes mEndianSwap into account so the explicit use of 
endian_swap is a duplicate and swaps the data back to the way it was 
before, so it is again invalid.
Taking out the test of mEndianSwap fixes the problem.

This was successful in displaying point data.  However when I tried to 
display polygon data, QGIS crashed.
It is crashing because geometry simplification logic is called which 
reads back the wkb data in the geometry which was saved in the original 
format and is still other-endian and causes chaos. The simplification 
code doesn't use QgsConstWkbPtr .

I tried changing the read function in QgsConstWkbPtr to copy the swapped 
data back into the wkb value but this didn't actually change the stored 
wkb.
     template<typename T> void read( T& v ) const
     {
       memcpy( &v, mP, sizeof( v ) );

       if ( mEndianSwap )
       {
         QgsApplication::endian_swap( v );
         memcpy(  mP, &v, sizeof( v ) );
       }
       mP += sizeof( v );
     }

Any suggestions?

On 1/26/2016 5:57 PM, Even Rouault wrote:
> Le mardi 26 janvier 2016 23:11:28, David Adler a écrit :
>> We just realized that DB2 on z/OS uses big-endian, first byte = x'00'.
>>
>> This doesn't appear to work with the QgsGeometry->fromWkb() function.
> Looking quickly it seems that indeed QgsGeometryFactory::geomFromWkb() assumes
> host ordering, but the QgsConstWkbPtr class used by the various
> Qgs{Geometry]v2::fromWkb() methods seem to support byte swapping when WKB
> endianness != host endianness. So you might just need to fix
> QgsGeometryFactory::geomFromWkb() to make that work.
>
>> We have other alternatives but were just wondering.
>>
>> ---
>> This email has been checked for viruses by Avast antivirus software.
>> https://www.avast.com/antivirus
>>
>> _______________________________________________
>> Qgis-developer mailing list
>> Qgis-developer at lists.osgeo.org
>> List info: http://lists.osgeo.org/mailman/listinfo/qgis-developer
>> Unsubscribe: http://lists.osgeo.org/mailman/listinfo/qgis-developer


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus