[Gc] Re: Re[2]: Patch for native support of long weakrefs

Zach Saw zach.saw at gmail.com
Thu Nov 15 01:32:05 PST 2012


Hi Ivan,

>
> 1. You latest patch requires minor fixing (no need to repost this patch,
just make another patch fixing this problem):
> DCL_LOCK_STATE declares any local variables needed by LOCK and UNLOCK, so
DCL_LOCK_STATE should be placed there LOCK/UNLOCK are (as I understand we
could deprecate/remove DCL_LOCK_STATE but not now).
>
> More refactoring:
> 2. GC_register_disappearing_link_inner: we can't move out locking from
this function, so please move argument checking back to it (just to avoid
code duplication)
>
> 3. GC_move_disappearing_link_inner: make lock/unlock out of this function
(so that GC_move_disappearing_link will do lock/call
GC_move_disappearing_link_inner/unlock/return res.

Why the inconsistency? Why don't we move all of the code into the inner
functions consistently? And when we introduce the new long weakref, it
won't have any code duplications.

> 4. GC_unregister_disappearing_link_inner: I think it's better to move
lock/unlock from it as well but, if so, it should return curr_dl (instead
of 1) or NULL, while GC_unregister_disappearing_link should do arg
checking, lock/unlock and GC_free/dl_set_next (for the latter operation, I
suggest do define a macro like GC_FREE_DL_ENTRY - as I recall one if your
submitted patches (with big defines) contained such macro).

If you agree with consistency argument above, this would be the same.

Zach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://napali.hpl.hp.com/pipermail/gc/attachments/20121115/4d6eadcd/attachment.htm


More information about the Gc mailing list