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

Reply via email to