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