Re: memory leaks in exif

Hello, On Fri, 29 Mar 2002, Marcus Boerger wrote: > i found memory leaks in ext/exif of 4.2.0. For all my test images it works but > i cannot execute run-tests.php with 4.2.0 - and i don't know why it allways > tells > me that i am in safe mode. So someone else has to test it and change the > *.phpt and test2.jpg > > I used lines starting with ! to mark what&why i changed. Can you merge this yourself to the branch? I've little understanding of all this. And what goes wrong with run-tests.php exactly? Derick > cvs -z3 -q diff ext\exif\exif.c (in directory S:\PHP_4_2_0\) > Index: ext/exif/exif.c > =================================================================== > RCS file: /repository/php4/ext/exif/exif.c,v > retrieving revision 1.52.2.2 > diff -u -r1.52.2.2 exif.c > --- ext/exif/exif.c 16 Mar 2002 20:02:12 -0000 1.52.2.2 > +++ ext/exif/exif.c 29 Mar 2002 01:18:26 -0000 > @@ -777,7 +777,7 @@ > if ( !info_value->value.s) { > info_value->length = 0; > php_error(E_WARNING, "Could not allocate > memory for thumbnail"); > - return; > + break; /* better return with "" instead of > possible casing problems */ > ! When running out of memory a break is better. > } > break; > > @@ -1732,7 +1732,7 @@ > ImageInfo->sections[ImageInfo->sections_count].Size = itemlen; > > Data = (uchar *)emalloc(itemlen+1); /* Add 1 to allow > sticking a 0 at the end. */ > - ImageInfo->sections[ImageInfo->sections_count].Data = Data; > + ImageInfo->sections[ImageInfo->sections_count++].Data = Data; > ! move counting up: important for return > > /* Store first two pre-read bytes. */ > Data[0] = (uchar)lh; > @@ -1743,7 +1743,6 @@ > php_error(E_WARNING, "error reading from file: > got=x%04X(=%d) != itemlen-2=x%04X(=%d)",got, got, itemlen-2, itemlen-2); > return FALSE; > } > - ImageInfo->sections_count += 1; > > #ifdef EXIF_DEBUG > php_error(E_NOTICE,"process section(x%02X=%s) @ x%04X + > x%04X(=%d)", marker, exif_get_markername(marker), fpos, itemlen, itemlen); > @@ -2074,7 +2073,7 @@ > int a; > > if ( ImageInfo->sections_count) { > - for (a=0;a<ImageInfo->sections_count-1;a++) { > + for (a=0;a<ImageInfo->sections_count;a++) { > ! missed one efree > efree(ImageInfo->sections[a].Data); > } > } > @@ -2092,6 +2091,7 @@ > > if (ImageInfo->FileName) efree(ImageInfo->FileName); > if (ImageInfo->Thumbnail) efree(ImageInfo->Thumbnail); > + if (ImageInfo->UserComment) efree(ImageInfo->UserComment); > ! missed one efree > for (i=0; i<SECTION_COUNT; i++) { > exif_free_image_info( ImageInfo, i); > } > @@ -2217,8 +2217,10 @@ > > ImageInfo.sections_found |= FOUND_COMPUTED;/* do not inform about > in debug*/ > > - if (ret==FALSE || array_init(return_value) == FAILURE || > (sections_needed && !(sections_needed&ImageInfo.sections_found))) { > + if (ret==FALSE || (sections_needed && > !(sections_needed&ImageInfo.sections_found) || array_init(return_value) == > FAILURE)) { > + /* array_init must be checked at last! otherwise the array > must be freed if a later test fails. */ > !possible missed efree on failure > php_exif_discard_imageinfo(&ImageInfo); > + if ( sections_str) efree( sections_str); > !missed one efree on failure > RETURN_FALSE; > } > > > > > --------->>> mailto:marcus.boerger@post.rwth-aachen.de > <<<------------ > "Wir sind allzumal Tiere unter Tieren, Kinder der Materie wie sie, > nur wehrloser. Doch da wir im Unterschied zu den Tieren wissen, > dass wir sterben muessen, wollen wir uns auf jenen Augenblick vorbereiten, > indem wir das Leben geniessen, das uns durch Zufall und vom Zufall gegeben > ist." > Umberto Eco, Die Insel des vorigen Tages > --------------->>> http://www.marcus-boerger.de > <<<------------------- > ---------->>> Tel. 0241 / 874 09-7 ### 0179 / 29 14 980 <<<---------- Derick Rethans --------------------------------------------------------------------- PHP: Scripting the Web - www.php.net - derick@php.net SRM: Sscript Running Manager - www.vl-srm.net --------------------------------------------------------------------- JDI Media Solutions - www.jdimedia.nl - d.rethans@jdimedia.nl Boulevard Heuvelink 102 - 6828 KT Arnhem - The Netherlands ---------------------------------------------------------------------