Hi, I’ve been spending the last few days reviewing some of the libtiff API and internals with a view towards enabling some stricter compiler warning flags to improve the quality of the codebase.
One thing which stood out was the use of tmsize_t for a lot of different functions, including memory allocation functions: libtiff/tif_predict.h:typedef int (*TIFFEncodeDecodeMethod)(TIFF *tif, uint8_t *buf, tmsize_t size); libtiff/tif_predict.h: tmsize_t stride; /* sample stride over data */ libtiff/tif_predict.h: tmsize_t rowsize; /* tile/strip row size */ libtiff/tiffio.h: * NB: tsize_t -> deprecated and replaced by tmsize_t libtiff/tiffio.h:typedef TIFF_SSIZE_T tmsize_t; libtiff/tiffio.h:#define TIFF_TMSIZE_T_MAX (tmsize_t)(SIZE_MAX >> 1) libtiff/tiffio.h:typedef tmsize_t tsize_t; /* i/o size in bytes */ libtiff/tiffio.h: typedef tmsize_t (*TIFFReadWriteProc)(thandle_t, void *, tmsize_t); libtiff/tiffio.h: extern void *_TIFFmalloc(tmsize_t s); libtiff/tiffio.h: extern void *_TIFFcalloc(tmsize_t nmemb, tmsize_t siz); libtiff/tiffio.h: extern void *_TIFFrealloc(void *p, tmsize_t s); libtiff/tiffio.h: extern void _TIFFmemset(void *p, int v, tmsize_t c); libtiff/tiffio.h: extern void _TIFFmemcpy(void *d, const void *s, tmsize_t c); libtiff/tiffio.h: extern int _TIFFmemcmp(const void *p1, const void *p2, tmsize_t c); libtiff/tiffio.h: extern tmsize_t TIFFScanlineSize(TIFF *tif); libtiff/tiffio.h: extern tmsize_t TIFFRasterScanlineSize(TIFF *tif); libtiff/tiffio.h: extern tmsize_t TIFFStripSize(TIFF *tif); libtiff/tiffio.h: extern tmsize_t TIFFRawStripSize(TIFF *tif, uint32_t strip); libtiff/tiffio.h: extern tmsize_t TIFFVStripSize(TIFF *tif, uint32_t nrows); libtiff/tiffio.h: extern tmsize_t TIFFTileRowSize(TIFF *tif); libtiff/tiffio.h: extern tmsize_t TIFFTileSize(TIFF *tif); libtiff/tiffio.h: extern tmsize_t TIFFVTileSize(TIFF *tif, uint32_t nrows); libtiff/tiffio.h: extern int TIFFReadBufferSetup(TIFF *tif, void *bp, tmsize_t size); libtiff/tiffio.h: extern int TIFFWriteBufferSetup(TIFF *tif, void *bp, tmsize_t size); libtiff/tiffio.h: tmsize_t max_single_mem_alloc); libtiff/tiffio.h: tmsize_t max_cumulated_mem_alloc); libtiff/tiffio.h: extern tmsize_t TIFFReadTile(TIFF *tif, void *buf, uint32_t x, uint32_t y, libtiff/tiffio.h: extern tmsize_t TIFFWriteTile(TIFF *tif, void *buf, uint32_t x, uint32_t y, libtiff/tiffio.h: extern tmsize_t TIFFReadEncodedStrip(TIFF *tif, uint32_t strip, void *buf, libtiff/tiffio.h: tmsize_t size); libtiff/tiffio.h: extern tmsize_t TIFFReadRawStrip(TIFF *tif, uint32_t strip, void *buf, libtiff/tiffio.h: tmsize_t size); libtiff/tiffio.h: extern tmsize_t TIFFReadEncodedTile(TIFF *tif, uint32_t tile, void *buf, libtiff/tiffio.h: tmsize_t size); libtiff/tiffio.h: extern tmsize_t TIFFReadRawTile(TIFF *tif, uint32_t tile, void *buf, libtiff/tiffio.h: tmsize_t size); libtiff/tiffio.h: tmsize_t insize, void *outbuf, libtiff/tiffio.h: tmsize_t outsize); libtiff/tiffio.h: extern tmsize_t TIFFWriteEncodedStrip(TIFF *tif, uint32_t strip, void *data, libtiff/tiffio.h: tmsize_t cc); libtiff/tiffio.h: extern tmsize_t TIFFWriteRawStrip(TIFF *tif, uint32_t strip, void *data, libtiff/tiffio.h: tmsize_t cc); libtiff/tiffio.h: extern tmsize_t TIFFWriteEncodedTile(TIFF *tif, uint32_t tile, void *data, libtiff/tiffio.h: tmsize_t cc); libtiff/tiffio.h: extern tmsize_t TIFFWriteRawTile(TIFF *tif, uint32_t tile, void *data, libtiff/tiffio.h: tmsize_t cc); libtiff/tiffio.h: extern void TIFFSwabArrayOfShort(uint16_t *wp, tmsize_t n); libtiff/tiffio.h: extern void TIFFSwabArrayOfTriples(uint8_t *tp, tmsize_t n); libtiff/tiffio.h: extern void TIFFSwabArrayOfLong(uint32_t *lp, tmsize_t n); libtiff/tiffio.h: extern void TIFFSwabArrayOfLong8(uint64_t *lp, tmsize_t n); libtiff/tiffio.h: extern void TIFFSwabArrayOfFloat(float *fp, tmsize_t n); libtiff/tiffio.h: extern void TIFFSwabArrayOfDouble(double *dp, tmsize_t n); libtiff/tiffio.h: extern void TIFFReverseBits(uint8_t *cp, tmsize_t n); libtiff/tiffiop.h:typedef int (*TIFFCodeMethod)(TIFF *tif, uint8_t *buf, tmsize_t size, libtiff/tiffiop.h:typedef void (*TIFFPostMethod)(TIFF *tif, uint8_t *buf, tmsize_t size); libtiff/tiffiop.h: tmsize_t tif_tilesize; /* # of bytes in a tile */ libtiff/tiffiop.h: tmsize_t tif_scanlinesize; /* # of bytes in a scanline */ libtiff/tiffiop.h: tmsize_t tif_scanlineskew; /* scanline skew for reading strips */ libtiff/tiffiop.h: tmsize_t tif_rawdatasize; /* # of bytes in raw data buffer */ libtiff/tiffiop.h: tmsize_t tif_rawdataoff; /* rawdata offset within strip */ libtiff/tiffiop.h: tmsize_t tif_rawdataloaded; /* amount of data in rawdata */ libtiff/tiffiop.h: tmsize_t tif_rawcc; /* bytes unread from raw buffer */ libtiff/tiffiop.h: tmsize_t tif_size; /* size of mapped file region (bytes, thus tmsize_t) */ libtiff/tiffiop.h: tmsize_t tif_max_single_mem_alloc; /* in bytes. 0 for unlimited */ libtiff/tiffiop.h: tmsize_t tif_max_cumulated_mem_alloc; /* in bytes. 0 for unlimited */ libtiff/tiffiop.h: tmsize_t tif_cur_cumulated_mem_alloc; /* in bytes */ libtiff/tiffiop.h: tmsize_t max_single_mem_alloc; /* in bytes. 0 for unlimited */ libtiff/tiffiop.h: tmsize_t max_cumulated_mem_alloc; /* in bytes. 0 for unlimited */ libtiff/tiffiop.h: extern int _TIFFNoRowEncode(TIFF *tif, uint8_t *pp, tmsize_t cc, libtiff/tiffiop.h: extern int _TIFFNoStripEncode(TIFF *tif, uint8_t *pp, tmsize_t cc, libtiff/tiffiop.h: extern int _TIFFNoTileEncode(TIFF *, uint8_t *pp, tmsize_t cc, uint16_t s); libtiff/tiffiop.h: extern int _TIFFNoRowDecode(TIFF *tif, uint8_t *pp, tmsize_t cc, libtiff/tiffiop.h: extern int _TIFFNoStripDecode(TIFF *tif, uint8_t *pp, tmsize_t cc, libtiff/tiffiop.h: extern int _TIFFNoTileDecode(TIFF *, uint8_t *pp, tmsize_t cc, uint16_t s); libtiff/tiffiop.h: extern void _TIFFNoPostDecode(TIFF *tif, uint8_t *buf, tmsize_t cc); libtiff/tiffiop.h: extern void _TIFFSwab16BitData(TIFF *tif, uint8_t *buf, tmsize_t cc); libtiff/tiffiop.h: extern void _TIFFSwab24BitData(TIFF *tif, uint8_t *buf, tmsize_t cc); libtiff/tiffiop.h: extern void _TIFFSwab32BitData(TIFF *tif, uint8_t *buf, tmsize_t cc); libtiff/tiffiop.h: extern void _TIFFSwab64BitData(TIFF *tif, uint8_t *buf, tmsize_t cc); libtiff/tiffiop.h: extern int _TIFFRewriteField(TIFF *, uint16_t, TIFFDataType, tmsize_t, libtiff/tiffiop.h: extern tmsize_t _TIFFMultiplySSize(TIFF *, tmsize_t, tmsize_t, libtiff/tiffiop.h: extern tmsize_t _TIFFCastUInt64ToSSize(TIFF *, uint64_t, const char *); libtiff/tiffiop.h: extern void *_TIFFCheckMalloc(TIFF *, tmsize_t, tmsize_t, const char *); libtiff/tiffiop.h: extern void *_TIFFCheckRealloc(TIFF *, void *, tmsize_t, tmsize_t, libtiff/tiffiop.h: extern tmsize_t _TIFFReadEncodedStripAndAllocBuffer(TIFF *tif, libtiff/tiffiop.h: tmsize_t bufsizetoalloc, libtiff/tiffiop.h: tmsize_t size_to_read); libtiff/tiffiop.h: extern tmsize_t _TIFFReadEncodedTileAndAllocBuffer(TIFF *tif, uint32_t tile, libtiff/tiffiop.h: tmsize_t bufsizetoalloc, libtiff/tiffiop.h: tmsize_t size_to_read); libtiff/tiffiop.h: extern tmsize_t _TIFFReadTileAndAllocBuffer(TIFF *tif, void **buf, libtiff/tiffiop.h: tmsize_t bufsizetoalloc, libtiff/tiffiop.h: extern void *_TIFFmallocExt(TIFF *tif, tmsize_t s); libtiff/tiffiop.h: extern void *_TIFFcallocExt(TIFF *tif, tmsize_t nmemb, tmsize_t siz); libtiff/tiffiop.h: extern void *_TIFFreallocExt(TIFF *tif, void *p, tmsize_t s); The reason these stand out is that there are an awful lot of static analysis warnings (over 400) relating to unnecessary (and potentially buggy) sign conversions, and most of these are due to the use of signed integers where unsigned would typically be used. And for the standard memory allocation functions, these all use size_t, not a signed size type like ssize_t. The same consideration may also apply to other functions where we are using the type to represent sizes which can not be negative, where unsigned would be more natural to use and in many cases all of the calculations are done with unsigned inputs. I was wondering if anyone could provide some historical context for why the API is this way. And as a followup, if it would be something we could consider changing to make the API and the internal implementation safer and cleaner. The next most significant are implicit narrowing conversions (just under 100). Over the next few weeks, I’m going to be looking at opening a series of merge requests to address various categories of static analysis warning in the codebase, plus enabling specific compiler warnings by default to check for them by default in the CI builds. The aim being to improve the quality and robustness of the codebase. Most of these are fairly trivial to implement, but questions such as the above really depend upon what the original intentions were and any compatibility constraints we have to factor in. I’m also going to take a look through all of the currently open merge requests. There are quite a few which are several years old, and I will re-review and close where it’s clear we will not be merging them in their current form. Thanks, Roger _______________________________________________ Tiff mailing list [email protected] https://lists.osgeo.org/mailman/listinfo/tiff
