[Magick-bugs] Patches to 6.3.3
Mark Stemm
mstemm at cloudmark.com
Fri Mar 23 11:00:46 PDT 2007
Hi, thanks for developing the library ImageMagick, it's incredibly
useful.
I've found 2 small memory leaks in wand/magick-image.c and
magick/stream.c and made some minor changes to the gif coder to make
reading and pinging images faster. The changes were originally made
relative to 6.3.1, but I updated the changes for 6.3.3 and these diffs
are relative to 6.3.3.
Here's a summary of the changes to each source file:
wand/magick-image.c had a leak in MagickGetImageColormapColor. message
is an allocated string, but ThrowWandException returns MagickFalse as a
side-effect, causing the allocated string to be leaked.
magick/stream.c had a leak in DestroyPixelStream. I didn't completely
understand this part of the code, but it looks like DestroyPixelStream
in magick/stream.c does many of the things that DestroyCacheInfo in
magick/cache.c does--one path is for a CacheInfo struct in an ImageInfo
struct, and the other is for a CacheInfo structs inside an Image struct?
However, DestroyPixelStream doesn't clean up cache_info->nexus_info
while DestroyCacheInfo does, which is a bug as GetCacheInfo allocates it
and is called both from magick/stream.c and magick/cache.c.
coders/gif.c inlines several small functions that are called repeatedly
by ReadBlobLZWByte. I also wrote an alternative to DecodeImage called
FasterDecodeImage that reads all the bytes for a given image within a
gif without actually uncompressing anything. This turns out to speed up
image pinging a lot.
Here are the diffs, as 3 separate attachments. Feel free to modify as
you wish or ignore if you don't want to accept the changes. If these
changes are incorrect, however, feedback would be appreciated so we can
back out or modify our local changes to 6.3.1.
Thanks!
--Mark
-------------- next part --------------
--- magick/stream.c.orig 2007-03-23 10:12:19.000000000 -0700
+++ magick/stream.c 2007-03-23 10:13:40.000000000 -0700
@@ -404,6 +404,17 @@ static void DestroyPixelStream(Image *im
RelinquishStreamPixels(cache_info);
RelinquishSemaphoreInfo(cache_info->semaphore);
cache_info->semaphore=DestroySemaphoreInfo(cache_info->semaphore);
+
+ if (cache_info->nexus_info != (NexusInfo *) NULL)
+ {
+ register long
+ id;
+
+ for (id=0; id < (long) cache_info->number_views; id++)
+ DestroyCacheNexus(cache_info,(unsigned long) id);
+ cache_info->nexus_info=(NexusInfo *)
+ RelinquishMagickMemory(cache_info->nexus_info);
+ }
cache_info=(CacheInfo *) RelinquishMagickMemory(cache_info);
}
-------------- next part --------------
--- coders/gif.c.orig 2007-03-23 09:59:30.000000000 -0700
+++ coders/gif.c 2007-03-23 10:08:49.000000000 -0700
@@ -120,7 +120,7 @@ typedef struct _LZWInfo
/*
Forward declarations.
*/
-static int
+static inline int
GetNextLZWCode(LZWInfo *,const unsigned long);
static MagickBooleanType
@@ -129,10 +129,10 @@ static MagickBooleanType
static ssize_t
ReadBlobBlock(Image *,unsigned char *);
-static long
+static inline long
PopLZWStack(LZWStack *);
-static void
+static inline void
PushLZWStack(LZWStack *,const unsigned long),
ResetLZWInfo(LZWInfo *);
@@ -141,6 +141,67 @@ static void
% %
% %
% %
+% F a s t e r D e c o d e I m a g e %
+% %
+% %
+% %
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%
+% FasterDecodeImage reads (but doesn't process) an image via GIF-coding.
+% It is used when pinging an image and simply reads the compressed image blocks
+% to advance the position within the blob.
+%
+% The format of the FasterDecodeImage method is:
+%
+% MagickBooleanType FasterDecodeImage(Image *image)
+%
+% A description of each parameter follows:
+%
+% o image: The address of a structure of type Image.
+%
+*/
+
+static MagickBooleanType FasterDecodeImage(Image *image)
+{
+ unsigned char data_size, block_len;
+
+ assert(image != (Image *) NULL);
+ assert(image->signature == MagickSignature);
+ if (image->debug != MagickFalse)
+ (void) LogMagickEvent(TraceEvent,GetMagickModule(),"%s",image->filename);
+
+ /* The first byte contains the number of bits needed to hold the
+ * decompressed pixel values */
+ if (ReadBlob(image,1,&data_size) != 1)
+ ThrowBinaryException(CorruptImageError,"CorruptImage",image->filename);
+
+ if (data_size > MaximumLZWBits)
+ ThrowBinaryException(CorruptImageError,"CorruptImage",image->filename);
+
+ /* Following that there are a series of (length,data) blocks. Read
+ * them until we find the terminator (length == 0) */
+
+ if (ReadBlob(image,1,&block_len) != 1)
+ ThrowBinaryException(CorruptImageError,"CorruptImage",image->filename);
+
+ while (block_len != 0) {
+ unsigned char c[256];
+
+ if (ReadBlob(image, block_len, c) != block_len)
+ ThrowBinaryException(CorruptImageError,"CorruptImage",image->filename);
+
+ if (ReadBlob(image, 1, &block_len) != 1)
+ ThrowBinaryException(CorruptImageError,"CorruptImage",image->filename);
+ }
+
+ return MagickTrue;
+}
+
+/*
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+% %
+% %
+% %
% D e c o d e I m a g e %
% %
% %
@@ -237,7 +298,7 @@ static LZWInfo *AcquireLZWInfo(Image *im
return(lzw_info);
}
-static int GetNextLZWCode(LZWInfo *lzw_info,const unsigned long bits)
+static inline int GetNextLZWCode(LZWInfo *lzw_info,const unsigned long bits)
{
int
code;
@@ -276,7 +337,7 @@ static int GetNextLZWCode(LZWInfo *lzw_i
return(code);
}
-static long PopLZWStack(LZWStack *stack_info)
+static inline long PopLZWStack(LZWStack *stack_info)
{
if (stack_info->index <= stack_info->codes)
return(-1);
@@ -284,7 +345,7 @@ static long PopLZWStack(LZWStack *stack_
return((long) *stack_info->index);
}
-static void PushLZWStack(LZWStack *stack_info,const unsigned long value)
+static inline void PushLZWStack(LZWStack *stack_info,const unsigned long value)
{
if (stack_info->index >= stack_info->top)
return;
@@ -362,7 +423,7 @@ static int ReadBlobLZWByte(LZWInfo *lzw_
return(PopLZWStack(lzw_info->stack));
}
-static void ResetLZWInfo(LZWInfo *lzw_info)
+static inline void ResetLZWInfo(LZWInfo *lzw_info)
{
lzw_info->bits=lzw_info->data_size+1;
lzw_info->maximum_code=1UL << lzw_info->bits;
@@ -1235,6 +1296,9 @@ static Image *ReadGIFImage(const ImageIn
/*
Decode image.
*/
+ if (image_info->ping)
+ status=FasterDecodeImage(image);
+ else
status=DecodeImage(image,(Quantum) opacity);
if ((image_info->ping == MagickFalse) && (status == MagickFalse))
{
-------------- next part --------------
--- wand/magick-image.c.orig 2007-03-23 10:14:36.000000000 -0700
+++ wand/magick-image.c 2007-03-23 10:16:53.000000000 -0700
@@ -3591,8 +3591,12 @@ WandExport MagickBooleanType MagickGetIm
*message;
message=GetExceptionMessage(errno);
- ThrowWandException(WandError,"Invalid colormap index",message);
+ // Replaced ThrowWandException with this, as ThrowWandException
+ // also returns MagickFalse, causing message to be leaked.
+ (void) ThrowMagickException(&wand->exception,GetMagickModule(),WandError, \
+ "Invalid colormap index","`%s'",message);
message=DestroyString(message);
+ return MagickFalse;
}
PixelSetQuantumColor(color,wand->images->colormap+index);
return(MagickTrue);
More information about the Magick-bugs
mailing list