Re: [Monetdb-developers] [Monetdb-pf-checkins] pathfinder/modules/pftijah pftijah.mx, , 1.228, 1.229
It's slightly more complex than this. At the MIL level you're right, you need to use wrd instead of int for bat counts. On the C level however, you should use the BUN type and also check for overflow when converting a wrd parameter to BUN (if sizeof(wrd) > sizeof(BUN) which is entirely possible).
And then the real bug in your checkin: you use OIDFMT to print some value of type wrd. This is incorrect. You must use LLFMT together with a cast to lng for that. (Better would be to use BUN and BUNFMT.)
I guess we could introduce a WRDFMT for printing wrd values, but we don't have that currently.
On 2009-05-14 19:42, Roberto Cornacchia wrote:
Update of /cvsroot/monetdb/pathfinder/modules/pftijah In directory 23jxhf1.ch3.sourceforge.com:/tmp/cvs-serv25786
Modified Files: pftijah.mx Log Message: This commit is a temporary fix for the incorrect definition of count() mil proc: count(BAT b) : int
This should be instead:
count(BAT b) : wrd
!! Note that the problem should be solved properly in algebra.mx, changing the definition of count() and fixing all related code.
In addition, this commit fixes some more incorrect usage of the int type, for example all bat offsets should be wrd and not int.
U pftijah.mx Index: pftijah.mx =================================================================== RCS file: /cvsroot/monetdb/pathfinder/modules/pftijah/pftijah.mx,v retrieving revision 1.228 retrieving revision 1.229 diff -u -d -r1.228 -r1.229 --- pftijah.mx 13 May 2009 07:35:03 -0000 1.228 +++ pftijah.mx 14 May 2009 17:42:09 -0000 1.229 @@ -117,7 +117,7 @@ : BAT[oid,oid] = CMDpf2tijah_node; "Translate Pathfinder node sequence to tijah node sequence"
-.COMMAND offsetindex( BAT[void,oid] offset_tid, int res_size) +.COMMAND offsetindex( BAT[void,oid] offset_tid, wrd res_size) : BAT[void,oid] = CMDoffsetindex;
"PARAMETERS: @@ -127,7 +127,7 @@ creates an offset index. [...972 lines suppressed...] BATiter tidprei, tidsizei, oldindexi, oldprei, oldsizei; oid *s_newindex, *lst_newindex, *s_newpre, *lst_newpre; @@ -5238,7 +5249,7 @@ newsize = BATnew(TYPE_void, TYPE_int, ressize); if (newsize == NULL) {
GDKerror("%s: could not allocate a result BAT[void,int] of size %d.\n", name, ressize);
GDKerror("%s: could not allocate a result BAT[void,int] of size "OIDFMT".\n", name, ressize); BATfree(res); BATfree(newpre); return(GDK_FAIL);@@ -5248,7 +5259,7 @@ newindex = BATnew(TYPE_void, TYPE_oid, ressize); if (newindex == NULL) {
GDKerror("%s: could not allocate a result BAT[void,oid] of size %d.\n", name, ressize);
GDKerror("%s: could not allocate a result BAT[void,oid] of size "OIDFMT".\n", name, ressize); BATfree(res); BATfree(newpre); BATfree(newsize);
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your production scanning environment may not be a perfect world - but thanks to Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700 Series Scanner you'll get full speed at 300 dpi even with all image processing features enabled. http://p.sf.net/sfu/kodak-com _______________________________________________ Monetdb-pf-checkins mailing list Monetdb-pf-checkins@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-pf-checkins
Sjoerd,
Thanks for checking.
You are right, in C the BUN type should be used. I have only made it slightly less incorrect by using wrd rather than int, but it still is incorrect. Actually, now that I have a closer look at that, many BATcount calls are still assigned to int variables, so I hadn't spotted all of them anyway. But I will convert all of them to BUN.
And of course I'll correct the wrong format string.
Roberto
On Thu, 2009-05-14 at 20:33 +0200, Sjoerd Mullender wrote:
It's slightly more complex than this. At the MIL level you're right, you need to use wrd instead of int for bat counts. On the C level however, you should use the BUN type and also check for overflow when converting a wrd parameter to BUN (if sizeof(wrd) > sizeof(BUN) which is entirely possible).
And then the real bug in your checkin: you use OIDFMT to print some value of type wrd. This is incorrect. You must use LLFMT together with a cast to lng for that. (Better would be to use BUN and BUNFMT.)
I guess we could introduce a WRDFMT for printing wrd values, but we don't have that currently.
On 2009-05-14 19:42, Roberto Cornacchia wrote:
Update of /cvsroot/monetdb/pathfinder/modules/pftijah In directory 23jxhf1.ch3.sourceforge.com:/tmp/cvs-serv25786
Modified Files: pftijah.mx Log Message: This commit is a temporary fix for the incorrect definition of count() mil proc: count(BAT b) : int
This should be instead:
count(BAT b) : wrd
!! Note that the problem should be solved properly in algebra.mx, changing the definition of count() and fixing all related code.
In addition, this commit fixes some more incorrect usage of the int type, for example all bat offsets should be wrd and not int.
U pftijah.mx Index: pftijah.mx =================================================================== RCS file: /cvsroot/monetdb/pathfinder/modules/pftijah/pftijah.mx,v retrieving revision 1.228 retrieving revision 1.229 diff -u -d -r1.228 -r1.229 --- pftijah.mx 13 May 2009 07:35:03 -0000 1.228 +++ pftijah.mx 14 May 2009 17:42:09 -0000 1.229 @@ -117,7 +117,7 @@ : BAT[oid,oid] = CMDpf2tijah_node; "Translate Pathfinder node sequence to tijah node sequence"
-.COMMAND offsetindex( BAT[void,oid] offset_tid, int res_size) +.COMMAND offsetindex( BAT[void,oid] offset_tid, wrd res_size) : BAT[void,oid] = CMDoffsetindex;
"PARAMETERS: @@ -127,7 +127,7 @@ creates an offset index. [...972 lines suppressed...] BATiter tidprei, tidsizei, oldindexi, oldprei, oldsizei; oid *s_newindex, *lst_newindex, *s_newpre, *lst_newpre; @@ -5238,7 +5249,7 @@ newsize = BATnew(TYPE_void, TYPE_int, ressize); if (newsize == NULL) {
GDKerror("%s: could not allocate a result BAT[void,int] of size %d.\n", name, ressize);
GDKerror("%s: could not allocate a result BAT[void,int] of size "OIDFMT".\n", name, ressize); BATfree(res); BATfree(newpre); return(GDK_FAIL);@@ -5248,7 +5259,7 @@ newindex = BATnew(TYPE_void, TYPE_oid, ressize); if (newindex == NULL) {
GDKerror("%s: could not allocate a result BAT[void,oid] of size %d.\n", name, ressize);
GDKerror("%s: could not allocate a result BAT[void,oid] of size "OIDFMT".\n", name, ressize); BATfree(res); BATfree(newpre); BATfree(newsize);
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your production scanning environment may not be a perfect world - but thanks to Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700 Series Scanner you'll get full speed at 300 dpi even with all image processing features enabled. http://p.sf.net/sfu/kodak-com _______________________________________________ Monetdb-pf-checkins mailing list Monetdb-pf-checkins@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-pf-checkins
-- Sjoerd Mullender
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your production scanning environment may not be a perfect world - but thanks to Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700 Series Scanner you'll get full speed at 300 dpi even with all image processing features enabled. http://p.sf.net/sfu/kodak-com _______________________________________________ Monetdb-developers mailing list Monetdb-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-developers
participants (2)
-
Roberto Cornacchia -
Sjoerd Mullender