No memcpy() in TIFFRead/WriteEncodedStrip/Tile in uncompressed case

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

No memcpy() in TIFFRead/WriteEncodedStrip/Tile in uncompressed case

Even Rouault-2
Hi,

When profiling I/O in the uncompressed case, I noticed that
TIFFRead/WriteEncodedStrip/Tile used memcpy() to copy the data between the
user provided buffer and the tif_rawdata intermediate buffer. I've committed an
improvement to avoid that :  https://trac.osgeo.org/gdal/changeset/34533
It works fine in my testing, but I'm not sure if I may have forgotten corner
cases in the use of the API that would need additional checks and you may be
aware of.

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/
Reply | Threaded
Open this post in threaded view
|

Re: No memcpy() in TIFFRead/WriteEncodedStrip/Tile in uncompressed case

Tom Lane-2
Even Rouault <[hidden email]> writes:
> When profiling I/O in the uncompressed case, I noticed that
> TIFFRead/WriteEncodedStrip/Tile used memcpy() to copy the data between the
> user provided buffer and the tif_rawdata intermediate buffer. I've committed an
> improvement to avoid that :  https://trac.osgeo.org/gdal/changeset/34533
> It works fine in my testing, but I'm not sure if I may have forgotten corner
> cases in the use of the API that would need additional checks and you may be
> aware of.

Seems like in the write case, you've added a new assumption that it's
okay for the library to clobber the user-supplied buffer contents.
I'm dubious about that, the more so since it only happens in some cases.

                        regards, tom lane
_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/
Reply | Threaded
Open this post in threaded view
|

Re: No memcpy() in TIFFRead/WriteEncodedStrip/Tile in uncompressed case

Even Rouault-2
Le dimanche 03 juillet 2016 19:20:13, Tom Lane a écrit :

> Even Rouault <[hidden email]> writes:
> > When profiling I/O in the uncompressed case, I noticed that
> > TIFFRead/WriteEncodedStrip/Tile used memcpy() to copy the data between
> > the user provided buffer and the tif_rawdata intermediate buffer. I've
> > committed an improvement to avoid that :
> > https://trac.osgeo.org/gdal/changeset/34533 It works fine in my testing,
> > but I'm not sure if I may have forgotten corner cases in the use of the
> > API that would need additional checks and you may be aware of.
>
> Seems like in the write case, you've added a new assumption that it's
> okay for the library to clobber the user-supplied buffer contents.
> I'm dubious about that, the more so since it only happens in some cases.

Tom,

Actually, the clobbering already happened (and is indeed a pain to deal with
on the application side). The

        /* swab if needed - note that source buffer will be altered */
        tif->tif_postdecode( tif, (uint8*) data, cc );

was already done in the general case :
https://github.com/vadz/libtiff/blob/Release-v4-0-6/libtiff/tif_write.c#L265 and
https://github.com/vadz/libtiff/blob/Release-v4-0-6/libtiff/tif_write.c#L445

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
Tiff mailing list: [hidden email]
http://lists.maptools.org/mailman/listinfo/tiff
http://www.remotesensing.org/libtiff/