[Monetdb-developers] [Monetdb-checkins] MonetDB5/src/mal mal_builder.mx, MonetDB_5-2, 1.26, 1.26.2.1

Niels Nes Niels.Nes at cwi.nl
Mon Oct 29 21:08:54 CET 2007


On Mon, Oct 29, 2007 at 08:43:53PM +0100, Sjoerd Mullender wrote:
> On 2007-10-29 18:37, Niels Nes wrote:
> > On Mon, Oct 29, 2007 at 02:43:42PM +0100, Stefan Manegold wrote:
> >> On Mon, Oct 29, 2007 at 12:38:15PM +0100, Stefan Manegold wrote:
> >>> On Mon, Oct 29, 2007 at 07:09:42AM +0000, Niels Nes wrote:
> >>>> Update of /cvsroot/monetdb/MonetDB5/src/mal
> >>>> In directory sc8-pr-cvs16.sourceforge.net:/tmp/cvs-serv18604/src/mal
> >>>>
> >>>> Modified Files:
> >>>>       Tag: MonetDB_5-2
> >>>> 	mal_builder.mx 
> >>>> Log Message:
> >>>> pushStr shouldn't do a GDKstrdup, as it may return an equal string from
> >>>> the constant cache, ie only if the returned variable holds the given string
> >>>> (poniters are equal) we do a string dup. This solves another memory leak.

I'm not changing the string. Just the assignment and comparison to .sval
should be of type char*. None are changing the value.
The clean fix is do the strdup and free it if its not used, which is
as bad I guess.

Niels
> >>>>
> >>>>
> >>>> Index: mal_builder.mx
> >>>> ===================================================================
> >>>> RCS file: /cvsroot/monetdb/MonetDB5/src/mal/mal_builder.mx,v
> >>>> retrieving revision 1.26
> >>>> retrieving revision 1.26.2.1
> >>>> diff -u -d -r1.26 -r1.26.2.1
> >>>> --- mal_builder.mx	28 Aug 2007 19:56:41 -0000	1.26
> >>>> +++ mal_builder.mx	29 Oct 2007 07:09:40 -0000	1.26.2.1
> >>>> @@ -260,10 +260,12 @@
> >>>>  	ValRecord cst;
> >>>>  
> >>>>  	cst.vtype= TYPE_str;
> >>>> -	cst.val.sval= GDKstrdup(val);
> >>>> +	cst.val.sval= val;
> >>> gcc -DHAVE_CONFIG_H -I. -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/source/MonetDB5/src/mal -I../.. -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/source/MonetDB5/src/mal -I../optimizer -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/source/MonetDB5/src/mal/../optimizer -I../scheduler -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/source/MonetDB5/src/mal/../scheduler -I../modules/kernel -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/source/MonetDB5/src/mal/../modules/kernel -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/prefix.--enable-strict_--disable-optimize_--enable-debug_--enable-assert/include/MonetDB -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/prefix.--enable-strict_--disable-optimize_--enable-debug_--enable-assert/include/MonetDB/common -I/ufs/manegold/_/scratch0/Monet/Testing/Stable/prefix.--enable-strict_--disable-optimize_--enable-debug_--enable-assert/include/MonetDB/gdk -DLIBMAL -Wall -Wextra -std=c99 -g -Werror-implicit-function-declaration -Werr
> 
> >  or
> >>   -
> >>>  Wpointer-arith -Wdeclaration-after-statement -Wundef -Wp,-D_FORTIFY_SOURCE=2 -D_REENTRANT -c mal_builder.c  -fPIC -DPIC -o .libs/libmal_la-mal_builder.o
> >>> cc1: warnings being treated as errors
> >>> /ufs/manegold/_/scratch0/Monet/Testing/Stable/source/MonetDB5/src/mal/mal_builder.mx: In function 'pushStr':
> >>> /ufs/manegold/_/scratch0/Monet/Testing/Stable/source/MonetDB5/src/mal/mal_builder.mx:263: warning: assignment discards qualifiers from pointer target type
> >>> make[5]: *** [libmal_la-mal_builder.lo] Error 1
> >> The following patch would fix the compilation --- don't know, though,
> >> whether the changed sematics are acceptable ...
> >>
> >> ========
> >> Index: MonetDB5/src/mal/mal_builder.mx
> >> ===================================================================
> >> RCS file: /cvsroot/monetdb/MonetDB5/src/mal/mal_builder.mx,v
> >> retrieving revision 1.26.2.1
> >> diff -u -0 -r1.26.2.1 mal_builder.mx
> >> --- MonetDB5/src/mal/mal_builder.mx     29 Oct 2007 07:09:40 -0000
> >> 1.26.2.1
> >> +++ MonetDB5/src/mal/mal_builder.mx     29 Oct 2007 13:42:15 -0000
> >> @@ -51 +51 @@
> >> -mal_export InstrPtr pushStr(MalBlkPtr mb, InstrPtr q, const char *val);
> >> +mal_export InstrPtr pushStr(MalBlkPtr mb, InstrPtr q, char *val);
> >> @@ -257 +257 @@
> >> -pushStr(MalBlkPtr mb, InstrPtr q, const char *val)
> >> +pushStr(MalBlkPtr mb, InstrPtr q, char *val)
> > 
> > This indeed changes the semantics.
> >> ========
> >>
> >> Stefan
> >>
> >>>>  	cst.len= strlen(cst.val.sval);
> >>>>  	_t = defConstant(mb,TYPE_str,&cst);
> >>>>  
> >>>> +	if (getConstant(mb,_t).val.sval == val) 
> > 
> > So I guess this works too.
> > if (getConstant(mb,_t).val.sval == (char*)val) 
> 
> I don't like this one.  The function prototype says it's a const char *,
> so I expect that it won't be changed.  If you override that inside the
> function, you're breaking the contract between caller and callee.
> 
> > Niels
> >>>> +		getConstant(mb,_t).val.sval = GDKstrdup(val);
> >>>>  	return pushArgument(mb, q, _t);
> >>>>  }
> >> -- 
> >> | Dr. Stefan Manegold | mailto:Stefan.Manegold at 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-4312       |
> >>
> >> -------------------------------------------------------------------------
> >> This SF.net email is sponsored by: Splunk Inc.
> >> Still grepping through log files to find problems?  Stop.
> >> Now Search log events and configuration files using AJAX and a browser.
> >> Download your FREE copy of Splunk now >> http://get.splunk.com/
> >> _______________________________________________
> >> Monetdb-developers mailing list
> >> Monetdb-developers at lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/monetdb-developers
> > 
> 
> 
> -- 
> Sjoerd Mullender

-- 

Niels Nes, Centre for Mathematics and Computer Science (CWI)
Kruislaan 413, 1098 SJ Amsterdam, The Netherlands
room C0.02,  phone ++31 20 592-4098, fax ++31 20 592-4312
url: http://www.cwi.nl/~niels   e-mail: Niels.Nes at cwi.nl




More information about the developers-list mailing list