On Mon, 30 Nov 2009, Mindaugas Kavaliauskas wrote: Hi,
> >2009-11-30 21:31 UTC+0200 Mindaugas Kavaliauskas (dbtopas/at/dbtopas.lt) > > + harbour/contrib/hbcairo > I'll write down a few issues I found in hbcairo development. > 1) #include "std.ch" generates compile time errors. Just like in Clipper. > 2) I'm a little confused how to implement GC pointers. So, I've > implemented two version in hbcairo/core.c. It can be selected > [not]using HB_USE_ITEM define. In current code the switch is not > defined and more "low level" version is used (actually, I even > forgot to test using HB_USE_ITEM). Are these GC pointer > implementations correct? A few subquestions are: No exactly. There are few problems which should be fixed. I'll commit the fix in a while. See details below. > 2a) hb_gcRefInc()/hb_gcRefDec() is required without HB_USE_ITEM. > This requires define _HB_API_INTERNAL_. Is this OK? First you wrongly used hb_gcRefDec() instead of hb_gcRefFree(). hb_gcRefDec() does not free allocated block. It only decrease number of reference counters. If you do not have any cross references then this block will be cleanly freed on next GC collect call. If you have such references then it may cause RT error ("Reference to freed block"). So you should use pair of functions: hb_gcRefInc()/hb_gcRefFree(). I do not think that hb_gcRefDec() is necessary for any 3-rd party code. It's not even used in current core code so probably I should remove it to not confuse users and maybe add: #define hb_gcRefDec hb_gcRefFree to hbapi.h. The new GC extensions opens door for some interesting extensions in none core code so maybe it's time to make hb_gcRefInc() and hb_gcRefFree() public functions though I'm not sure how many people will have enough knowledge to use it without breaking some internals. Fist I would like to inform everybody that these functions _CANNOT_ be used as workaround for some problems with code using GC memory blocks. They can be used _ONLY_ to improve a little bit performance in code which should work also without them. > 2b) In function CAIRO_PATH_ITERATOR_CREATE() new item is created > using: pIterator->pPath = hb_itemNew( hb_param( 1, HB_IT_POINTER ) > ); > 2c) Does pIterator->pPath need to be unlocked in case 2b? I will mark it! If you do not unlock it then it will be always visible for GC and GC will also mark it as used and execute its mark function so your call to mark this function is unimportant but see below. > 2d) If it remains locked, then hb_gcGripMark( pIterator->pPath ) in > mark function is not necessary? The answer for your question is yes. All locked memory blocks are automatically marked as used and GC executes mark function for each of them. But you should _NEVER_ use hb_gcGripMark() - this is really low level HVM only function which is used in some _very_ special cases and calling this function may cause very serious problems, i.e. used in such context it may cause infinite mark loop in GC. You should use hb_gcMark instead. In fact for locked blocks it can return without any action but nothing wrong will happen in such case because locked blocks are marked anyway. > 2e) Can I be sure that inside CAIRO_PATH_ITERATOR_NEXT() pPath is a > valid (not destroyed path)? (both with and without HB_USE_ITEM) In valid code yes. GC has protection against multiple calling of destructor for the same GC memory block. Unlike in xHarbour in Harbour this protection always work. Anyhow there is one exception for above code intentionally implemented. It can be activated only in one case by buggy user .prg code when items are freed by GC. It's possible that inside destructor user will copy item marked to destroy to some location which will be accessible after GC pass, i.e. to some static or public variable. In such case GC cannot free such memory block because somewhere is stored reference to this block so it generates RT error (EG_DESTRUCTOR/1301: "Reference to freed block") and reattach this memory block to list of GC blocks to avoid GPF when this reference will be accessed. It means that it's good to make same at least basic protection inside destructor which allows to "reuse" GC block for which destructor is activated. It's not necessary to make it fully functional. Just simply add some basic protection against GPF, i.e. in hb_codeblockGarbageDelete() you may find this line: pCBlock->pCode = ( BYTE * ) s_pCode; which is such protection for codeblocks so if user inside object destructor copy some codeblocks which are marked to destroy with the object to some public variable and then after above RT error will try to use it then it will cause GPF though such blocks will not work as original one (see s_pCode body). If you look for array, object, hash, codeblock destructors then you will find that each of them leaves GC block ready to reuse. Checking and clearing pIterator->pPath inside hb_cairo_path_iterator_destructor() is quite good protection for such situations anyhow it will be good if you also check if pPath after code like: #ifdef HB_USE_ITEM pPath = hb_itemGetPtr( pIterator->pPath ); #else pPath = pIterator->pPath; #endif is not NULL. > 3)What is correct way to code: > pCairo = hb_cairo_par( 1 ); // generate RTE if parameter is wrong > if( pData ) > cairo_something( pCairo ); > or this is also ok?: > cairo_something( hb_cairo_par( 1 ) ); Sorry but I do not understand what you are asking for here. > 4) Should implementation of procedures end with hb_ret(); ? In most of cases not but it depends on context. Sometimes hb_ret() can be used to overwrite return value set by some other code in current function or by some other HVM functions executed from current function without some code to safe and restore return value like hb_vmRequestReenter()/ hb_vmRequestRestore(). In some cases it can be used also to clean return value left by function called as procedure just before the current one. In such context hb_ret() is used in HB_GCALL() function (see my comment in garbage.c) In general when HVM (it's also Clipper VM behavior) calls function to retrieve its result, i.e.: x := somefunc() or qout( somefunc() ) then it always set NIL to return value just before executing the function and this is the default return value. But if it calls any function without retrieving return value, i.e.: somefunc() then it does nothing with existing return value. In some cases it can be used in quite interesting way even in .prg code. This code illustrates it: proc main() ? p( 1 ) ? p( 2 ) ? p( 3 ) ? p( 4 ) ? p( 0 ) proc p( x ) if x == 1 f1() elseif x == 2 f2() elseif x == 3 f3() elseif x == 4 f4() else f0() endif return func f0(); return NIL func f1(); return procname() func f2(); return procname() func f3(); return procname() func f4(); return procname() Please note that inside p() function f0() works like hb_ret() in C code and can be eliminated because in ? p( 0 ) HVM clears return item just before executing p() best regards, Przemek _______________________________________________ Harbour mailing list (attachment size limit: 40KB) Harbour@harbour-project.org http://lists.harbour-project.org/mailman/listinfo/harbour