From 88148318eb6b001289a48cb7380acaf44b183eb1 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 30 Jul 2016 10:51:54 +0100 Subject: [PATCH] fix performance regression the extra check on bandfmt in sizeof() in 8.3.2 was causing some performance problems ... move the check to file read, so we only do it once per image, not once per pixel or scanline thanks Lovell! --- ChangeLog | 4 ++++ TODO | 7 ------- configure.ac | 6 +++--- libvips/include/vips/header.h | 1 + libvips/include/vips/image.h | 3 ++- libvips/iofuncs/header.c | 17 +++++++++++++++++ libvips/iofuncs/vips.c | 14 +++++++++++++- 7 files changed, 40 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index d7981499..18931555 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +30/7/16 started 8.3.3 +- fix performance regression in 8.3.2, thanks Lovell +- yet more robust vips file reading + 18/5/16 started 8.3.2 - more robust vips image reading - more robust tiff read [Matt Richards] diff --git a/TODO b/TODO index 1f1a7cfc..cf0dcd7a 100644 --- a/TODO +++ b/TODO @@ -1,10 +1,3 @@ -- the gif tests in the suite sometimes fail with giflib5 because of an - uninitialized struct in giflib, see - - https://sourceforge.net/p/giflib/bugs/94/ - - sadly ubuntu 16.04 only comes with giflib5, and giflib5 is currently broken - - I like the new int mask creator in reducev, can we use it in im_vips2imask() as well? diff --git a/configure.ac b/configure.ac index 14424b8a..929b9b53 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # also update the version number in the m4 macros below -AC_INIT([vips], [8.3.2], [vipsip@jiscmail.ac.uk]) +AC_INIT([vips], [8.3.3], [vipsip@jiscmail.ac.uk]) # required for gobject-introspection AC_PREREQ(2.62) @@ -18,7 +18,7 @@ AC_CONFIG_MACRO_DIR([m4]) # user-visible library versioning m4_define([vips_major_version], [8]) m4_define([vips_minor_version], [3]) -m4_define([vips_micro_version], [2]) +m4_define([vips_micro_version], [3]) m4_define([vips_version], [vips_major_version.vips_minor_version.vips_micro_version]) @@ -38,7 +38,7 @@ VIPS_VERSION_STRING=$VIPS_VERSION-`date` # binary interface changes not backwards compatible?: reset age to 0 LIBRARY_CURRENT=46 -LIBRARY_REVISION=2 +LIBRARY_REVISION=3 LIBRARY_AGE=4 # patched into include/vips/version.h diff --git a/libvips/include/vips/header.h b/libvips/include/vips/header.h index 8831f933..ba3b94a3 100644 --- a/libvips/include/vips/header.h +++ b/libvips/include/vips/header.h @@ -109,6 +109,7 @@ extern "C" { #define VIPS_META_LOADER "vips-loader" guint64 vips_format_sizeof( VipsBandFormat format ); +guint64 vips_format_sizeof_unsafe( VipsBandFormat format ); int vips_image_get_width( const VipsImage *image ); int vips_image_get_height( const VipsImage *image ); diff --git a/libvips/include/vips/image.h b/libvips/include/vips/image.h index ee8d7a6b..d1228632 100644 --- a/libvips/include/vips/image.h +++ b/libvips/include/vips/image.h @@ -361,10 +361,11 @@ GType vips_image_get_type( void ); /* Has to be guint64 and not size_t/off_t since we have to be able to address * huge images on platforms with 32-bit files. */ + /* Pixel address calculation macros. */ #define VIPS_IMAGE_SIZEOF_ELEMENT( I ) \ - (vips_format_sizeof((I)->BandFmt)) + (vips_format_sizeof_unsafe((I)->BandFmt)) #define VIPS_IMAGE_SIZEOF_PEL( I ) \ (VIPS_IMAGE_SIZEOF_ELEMENT( I ) * (I)->Bands) #define VIPS_IMAGE_SIZEOF_LINE( I ) \ diff --git a/libvips/iofuncs/header.c b/libvips/iofuncs/header.c index 8d748507..19307cd9 100644 --- a/libvips/iofuncs/header.c +++ b/libvips/iofuncs/header.c @@ -201,6 +201,23 @@ vips_format_sizeof( VipsBandFormat format ) return( vips__image_sizeof_bandformat[format] ); } +/** + * vips_format_sizeof_unsafe: (skip) + * @format: format type + * + * A fast but dangerous version of vips_format_sizeof(). You must have + * previously range-checked @format or you'll crash. + * + * Returns: number of bytes for a band format. + */ +guint64 +vips_format_sizeof_unsafe( VipsBandFormat format ) +{ + g_assert( 0 <= format && format <= VIPS_FORMAT_DPCOMPLEX ); + + return( vips__image_sizeof_bandformat[format] ); +} + #ifdef DEBUG /* Check that this meta is on the hash table. */ diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 9ed6b814..92dfb144 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -1,4 +1,4 @@ -/* Read and write a vips file +/* Read and write a vips file. * * 22/5/08 * - from im_open.c, im_openin.c, im_desc_hd.c, im_readhist.c, @@ -330,6 +330,18 @@ vips__read_header_bytes( VipsImage *im, unsigned char *from ) im->Xres = im->Xres_float; im->Yres = im->Yres_float; + /* Some protection against malicious files. We also check predicted + * (based on these values) against real file length, see below. + */ + im->Xsize = VIPS_CLIP( 1, im->Xsize, VIPS_MAX_COORD ); + im->Ysize = VIPS_CLIP( 1, im->Ysize, VIPS_MAX_COORD ); + im->Bands = VIPS_CLIP( 1, im->Bands, VIPS_MAX_COORD ); + im->BandFmt = VIPS_CLIP( 0, im->BandFmt, VIPS_FORMAT_LAST - 1 ); + + /* Type, Coding, Offset, Res, etc. don't affect vips file layout, just + * pixel interpretation, don't clip them. + */ + return( 0 ); }