Re: Corrupted heap

[Fred Kiefer]

I applied your patch, leaving out the GSTEP_PROGRESSIVE_CODEC bit of it.
I really would like to see platfrom specific code like this removed from
GNUstep gui, but doing so now could break applications on Cygwin that
rely on the current behaviour. (Are there any?)
What we should have here is code that tests whether this specific
feature is present in the used JPEG library or not.

Fred

Am 17.03.2010 21:59, schrieb Vincent Richomme:
> On Wed, 17 Mar 2010 21:42:21 +0100, Fred Kiefer <address@hidden> wrote:
>> Am 17.03.2010 20:26, schrieb Adam Fedor:
>>>
>>> On Mar 17, 2010, at 12:34 PM, Vincent Richomme wrote:
>>>>
>>>>
>>>> /* Return YES if this looks like a JPEG. */
>>>> + (BOOL) _bitmapIsJPEG: (NSData *)imageData
>>>> {
>>>>  struct jpeg_decompress_struct  cinfo;
>>>>
>>>> ... BLABLA ...
>>>>
>>>> // establish return context for error handling
>>>>  if (setjmp(jerrMgr.setjmpBuffer))
>>>>    {
>>>>      gs_jpeg_memory_src_destroy(&cinfo);
>>>>      jpeg_destroy_decompress(&cinfo);
>>>>      return NO;
>>>>    }
>>>>
>>>> ... BLABLA ...
>>>>
>>>>   return YES;
>>>> }
>>>>
>>>> Are you sure you need to call free here because I suppose &cinfo->src
>>>> is
>>>> not yet valid.
>>>
>>> That looks wrong to me as well.
>>>
>> In itself this could be correct. setjmp() will return non-zero when
>> called via longjmp, that is, this is the error case, where we need to
>> clean up. Still the code isn't writen in a save way. We should first
>> bring the structure in a defined state, we never know at what point an
>> error will happen.
> 
> 
> Yes that's why I have added a memset where this pattern was used :
> 
> ===================================================================
> --- Source/NSBitmapImageRep+JPEG.m      (revision 29981)
> +++ Source/NSBitmapImageRep+JPEG.m      (working copy)
> @@ -56,7 +56,7 @@
>  #include <jpeglib.h>
>  #if defined(__CYGWIN__)
>  /* Cygwin uses a patched jpeg */
> -#define GSTEP_PROGRESSIVE_CODEC
> +//#define GSTEP_PROGRESSIVE_CODEC
>  #endif
> 
>  #include <setjmp.h>
> @@ -350,6 +350,8 @@
>    struct jpeg_decompress_struct  cinfo;
>    struct gs_jpeg_error_mgr  jerrMgr;
> 
> +  memset((void*)&cinfo, 0, sizeof(struct jpeg_decompress_struct));
> +
>    /* Be sure imageData contains data */
>    if (![imageData length])
>      {
> @@ -400,6 +402,8 @@
>    if (!(self = [super init]))
>      return nil;
> 
> +  memset((void*)&cinfo, 0, sizeof(struct jpeg_decompress_struct));
> +
>    /* Establish the our custom error handler */
>    gs_jpeg_error_mgr_init(&jerrMgr);
>    cinfo.err = jpeg_std_error(&jerrMgr.parent);
> @@ -549,6 +553,8 @@
>        return nil;
>      }
> 
> +  memset((void*)&cinfo, 0, sizeof(struct jpeg_decompress_struct));
> +
>    imageSource = [self bitmapData];
>    sPP = [self samplesPerPixel];
>    width = [self size].width;