[Monetdb-developers] [Monetdb-checkins] MonetDB5/src/modules/mal pma.mx, , 1.7, 1.8

Stefan Manegold Stefan.Manegold at cwi.nl
Fri Dec 4 16:41:19 CET 2009


Nan,

in addition to Sjoerd's comments, I noticed that your approach to
solving/avoiding some of the type mismatch complaints on Windows
is to add an explicit cast, in most cases from type oid to type int.
The latter is 32-bit (signed) on all platforms, while the former can be
either 32-bit or 64-bit (unsigned, but only 31/63 bits are actually used).
In case oid is 64-(63-)bit, casting to int might result in an overflow;
I did not see any checks or assertions to ensure that such overflows cannot
happen or are detected properly.

As far as I can see, most of the cases deal with variable that (explicitly
or implicitly) represent the number of BUNs in a BAT. For that purpose, you
should better use type BUN in C code and :wrd in MAL, instead of mixing oid
& int. :wrd is 32-bit on 32-bit systems and 64-bit on 64-bit systems
(signed); BUN is large enough to hold the max. number of BUNs allowed per
BAT, i.e., 32-bit on 32-bit systems and 64-bit systems with 32-bit OIDs, and
64-bit on 64-bit systems with 64-bit OIDs (unsigned, but only 31/63 bits are
used).

Let me know, in case you need more advice about the MonetDB type system and
how to use it.

Stefan

On Fri, Dec 04, 2009 at 10:27:01AM +0100, Sjoerd Mullender wrote:
> I noticed that you return constant strings when an error occurs.  This
> doesn't work since the interpreter expects that strings are allocated
> with GDKmalloc (i.e., it calls GDKfree on the error messages).
> 
> When you want to return an error, use the macro throw() (see numerous
> examples in the code).
> 
> Also, I noticed that there are calls to PMAnew in your code that don't
> check the return value (which could be an allocated string per the
> above), so there is a potential memory leak here.  (Plus, when PMAnew
> fails for whatever reason, you may not want to continue.)
> 
> This latter remark might be true for other functions.  I didn't check.
> 
> Tang Nan wrote:
> > Update of /cvsroot/monetdb/MonetDB5/src/modules/mal
> > In directory sfp-cvsdas-1.v30.ch3.sourceforge.com:/tmp/cvs-serv28068/src/modules/mal
> > 
> > Modified Files:
> > 	pma.mx 
> > Log Message:
> > fix some bugs about type conversion failure on Windows
> > 
> > 
> > Index: pma.mx
> > ===================================================================
> > RCS file: /cvsroot/monetdb/MonetDB5/src/modules/mal/pma.mx,v
> > retrieving revision 1.7
> > retrieving revision 1.8
> > diff -u -d -r1.7 -r1.8
> > --- pma.mx	2 Nov 2009 16:53:38 -0000	1.7
> > +++ pma.mx	4 Dec 2009 08:52:46 -0000	1.8
> > @@ -75,10 +75,6 @@
> >  @:pma_mal(lng)@
> >  @:pma_mal(flt)@
> >  @:pma_mal(dbl)@
> > -@(
> > -StM: disabled as the code clearly cannot handle type str (yet?)
> > -@:pma_mal(str)@
> > -@)
> >  
> >  @+ PMA API
> >  @h
> > @@ -115,10 +111,6 @@
> >  @:pma_decl(lng)@
> >  @:pma_decl(flt)@
> >  @:pma_decl(dbl)@
> > -@(
> > -StM: disabled as the code clearly cannot handle type str (yet?)
> > -@:pma_decl(str)@
> > -@)
> >  #endif	/* _PMA_H_ */
> >  
> >  @+ PMA implementation
> > @@ -155,10 +147,6 @@
> >  	for (cnt = PMA_SEG; cnt < *sz; cnt <<= 1);	/* figure out the size */
> >  	b = BATnew(TYPE_void, *tpe, cnt);
> >  	if (b) {
> > -@(
> > -StM: disabled as the code clearly cannot handle other than fixed-size linear types (yet?)
> > -		var_t nil_off;
> > -@)
> >  		switch(ATOMstorage(*tpe)) {
> >  		case TYPE_chr: @:fillnill(chr, chr_nil)@
> >  		case TYPE_bte: @:fillnill(bte, bte_nil)@
> > @@ -168,15 +156,8 @@
> >  		case TYPE_flt: @:fillnill(flt, flt_nil)@
> >  		case TYPE_dbl: @:fillnill(dbl, dbl_nil)@
> >  		case TYPE_ptr: @:fillnill(ptr, ptr_nil)@
> > -		default:	
> > -			BBPunfix(b->batCacheid);
> > -			return "PMAnew: type not supported (yet?)";
> > -@(
> > -StM: disabled as the code clearly cannot handle other than fixed-size linear types (yet?)
> > -			       BUNins(b, NULL, (ptr)str_nil, FALSE);
> > -                	       nil_off = *(var_t*) Tloc(b, BUNfirst(b));
> > -		               @:fillnill(wrd, nil_off)@
> > -@)
> > +		default:		BBPunfix(b->batCacheid);
> > +						return "PMAnew: type not supported (yet?)";
> >  		}
> >  		BATsetcount(b, cnt);
> >  		b->tsorted = 0;
> > @@ -417,7 +398,7 @@
> >  	bat ret;
> >  	BAT *b, *bn;
> >  	@1 *base, *basen;
> > -	(void)dummy;
> > +	dummy = 0;
> >  	b = BATdescriptor(*bid);
> >  	if (b == NULL) return "PMAdel: illegal bat parameter";
> >  	psz = BATcount(b);
> > @@ -725,9 +706,9 @@
> >  pma_export str
> >  PMAbulkins_ at 1(oid *pos_res, bat *bid, oid *pos, bat *ibid) {
> >  	bit fgw, fg;
> > -	int tpe, size;
> > +	int tpe, size, slen;
> >  	oid i, j, k, ht, sz, isz, psz, cnt, icnt, lv, wsz, snum, scl, scr, wbeg, mpos, cur;
> > -	dbl dens, dent, step, dstep, slen;
> > +	dbl dens, dent, step, dstep;
> >  	bat ret;
> >  	BAT *b, *bi, *bn;
> >  	@1 *base, *basei, *basen;
> > @@ -795,7 +776,7 @@
> >  					cur = i;
> >  				}
> >  			} else {
> > -				if (fg && (i - cur >= slen)) {
> > +				if (fg && ((int)(i - cur) >= slen)) {
> >  					base[cur] = base[i];
> >  					base[i] = (@1)@1_nil;
> >  					cur += slen;
> > @@ -815,7 +796,7 @@
> >  					cur = i;
> >  				}
> >  			} else {
> > -				if (fg && (cur - i >= slen)) {
> > +				if (fg && ((int)(cur - i) >= slen)) {
> >  					base[cur] = base[i];
> >  					base[i] = (@1)@1_nil;
> >  					cur -= slen;
> > @@ -839,7 +820,7 @@
> >  		while (TaoMin * size < cnt + icnt) size <<= 1;
> >  		dstep = (dbl)size / (cnt + icnt);
> >  		slen = ceil(dstep);
> > -		if ((cnt + icnt) * slen > size) slen--;
> > +		if ((int)(cnt + icnt) * slen > size) slen--;
> >  		tpe = TYPE_ at 1;
> >  		PMAnew(&ret, &tpe, &size);
> >  		bn =  BATdescriptor(ret);
> > @@ -877,7 +858,7 @@
> >  	BAT *b;
> >  	oid lo, hi, lo_res, hi_res, pos_res;
> >  	int i;
> > -	(void)dummy;
> > +	dummy = 0;
> >  	b = BATdescriptor(*bid);
> >  	if (b == NULL) return "PMAfnd: illegal bat parameter";
> >  	for (i = 0; i < *num; i++) {
> > @@ -916,7 +897,3 @@
> >  @:pma_impl(lng)@
> >  @:pma_impl(flt)@
> >  @:pma_impl(dbl)@
> > -@(
> > -StM: disabled as the code clearly cannot handle type str (yet?)
> > -@:pma_impl(str)@
> > -@)
> > 
> > 
> > ------------------------------------------------------------------------------
> > Join us December 9, 2009 for the Red Hat Virtual Experience,
> > a free event focused on virtualization and cloud computing. 
> > Attend in-depth sessions from your desk. Your couch. Anywhere.
> > http://p.sf.net/sfu/redhat-sfdev2dev
> > _______________________________________________
> > Monetdb-checkins mailing list
> > Monetdb-checkins at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/monetdb-checkins
> 
> 
> -- 
> Sjoerd Mullender
> 



> ------------------------------------------------------------------------------
> Join us December 9, 2009 for the Red Hat Virtual Experience,
> a free event focused on virtualization and cloud computing. 
> Attend in-depth sessions from your desk. Your couch. Anywhere.
> http://p.sf.net/sfu/redhat-sfdev2dev
> _______________________________________________
> Monetdb-checkins mailing list
> Monetdb-checkins at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/monetdb-checkins


-- 
| 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       |




More information about the developers-list mailing list