On Sat, Mar 20, 2010 at 02:56:11PM +0100, Martin Kersten wrote:
Stefan Manegold wrote:
On Sat, Mar 20, 2010 at 12:53:12PM +0000, Martin Kersten wrote:
Update of /cvsroot/monetdb/MonetDB5/src/modules/kernel In directory sfp-cvsdas-1.v30.ch3.sourceforge.com:/tmp/cvs-serv10537
Modified Files: Tag: Feb2010 batcast.mx Log Message: Backport of possible dangerous double frees. No roll forward needed.
While your revised code looks indeed more efficient to me (as it allows ATOMfromstr() --- well, at least strFromStr(); I cannot tell about other "extern" (varsized) atoms --- to reuse a allocated temporary buffer), I wonder whether/why the original experienced double frees? The implicit contract in gdk_atoms is to use v+len, i.e. pass a pre-alloced buffer and realloc upon need. Just freeing v without resetting it to NULL is bound to give problems with larger values becoming stored.
Setting v=NULL would have been the other option.
v was declared as ptr v = NULL; with in the look body, and hence should have been initialized with each iteration, shouldn't it? Stefan
Called with v==NULL, ATOMfromstr() --- again, I refer only to strFromStr(); FromStr functions for other "extern" (varsized) atoms might behave differently; IMHO, we don't have any "contract" or "policies" for FromStr() implementations --- would allocate a buffer for the result, ATOMput() copies the value from the buffer to the BAT heap, and then the buffer is (was) freed.
Looks very sound to me.
What am I missing?
I would be thankful for any enlightment.
Stefan
Index: batcast.mx =================================================================== RCS file: /cvsroot/monetdb/MonetDB5/src/modules/kernel/batcast.mx,v retrieving revision 1.31.2.7 retrieving revision 1.31.2.8 diff -u -d -r1.31.2.7 -r1.31.2.8 --- batcast.mx 18 Feb 2010 01:04:05 -0000 1.31.2.7 +++ batcast.mx 20 Mar 2010 12:53:10 -0000 1.31.2.8 @@ -248,6 +248,8 @@ BAT *b,*bn; BATiter bi; BUN p, q, r = 0; + ptr v = NULL; + int len = 0;
@:getBATdescriptor(bid, b, "batcalc.@1")@ @:voidresultBAT(TYPE_@1,"batcalc.@1")@ @@ -255,15 +257,12 @@
BATaccessBegin(b, USE_TAIL, MMAP_SEQUENTIAL); BATloop(b, p, q) { - ptr v = NULL; - int len = 0; - ATOMfromstr(TYPE_@1, &v, &len, (char *) BUNtvar(bi, p)); ATOMput(TYPE_@1, bn->T->vheap, Tloc(bn, r), v); - if (ATOMextern(TYPE_@1)) - GDKfree(v); r++; } + if (ATOMextern(TYPE_@1)) + GDKfree(v); BATsetcount(bn, BATcount(b)); bunins_failed: BATaccessEnd(b, USE_TAIL, MMAP_SEQUENTIAL);
------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Monetdb-checkins mailing list Monetdb-checkins@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-checkins
------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Monetdb-checkins mailing list Monetdb-checkins@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-checkins
-- | Dr. Stefan Manegold | mailto:Stefan.Manegold@cwi.nl | | CWI, P.O.Box 94079 | http://www.cwi.nl/~manegold/ | | 1090 GB Amsterdam | Tel.: +31 (20) 592-4212 | | The Netherlands | Fax : +31 (20) 592-4199 |