[Monetdb-developers] [Monetdb-checkins] MonetDB5/src/modules/kernel batcast.mx, Feb2010, 1.31.2.7, 1.31.2.8

Martin Kersten Martin.Kersten at cwi.nl
Sat Mar 20 14:56:11 CET 2010


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.
> 
> 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. at 1")@
>>  	@:voidresultBAT(TYPE_ at 1,"batcalc. at 1")@
>> @@ -255,15 +257,12 @@
>>  
>>  	BATaccessBegin(b, USE_TAIL, MMAP_SEQUENTIAL);
>>  	BATloop(b, p, q) {
>> -		ptr v = NULL; 
>> -		int len = 0;
>> -
>>  		ATOMfromstr(TYPE_ at 1, &v, &len, (char *) BUNtvar(bi, p));
>>  		ATOMput(TYPE_ at 1, bn->T->vheap, Tloc(bn, r), v);
>> -		if (ATOMextern(TYPE_ at 1))
>> -			GDKfree(v);
>>  		r++;
>>  	}
>> +	if (ATOMextern(TYPE_ at 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 at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/monetdb-checkins
>>
> 





More information about the developers-list mailing list