Additional comments (third reply mail).
11) In ImmModel::addNoDanglingReferences(ObjectInfo *object)
Comment should explain that this function extracts all NO_DANGLING
references that exist in "object"
and insert them into sReverseRefObjectMap.
Thats with 'object' as source and the object matching the dn in the
no-dangling attribute as target.
This is otherwise a fairly simple and clean function.
-------------------------------------------
12) In ImmModel::removeNoDanglingReferences(ObjectInfo *object,
ObjectInfo *afim, bool removeAll)
The leading comment must be improved.
The function does not operate on the argument "object", it operates on
sReverseRefObjectMMap.
I think the 'removeAll' argument could probably be called
removeRefsToObject ?
Classic object-orientation would have made sense here but I am no
insisting on that now.
This function is not quite symetric with addNoDanglingReferences.
So the comment should be changed say something like:
/*
* Delete from sReverseRefObjectMMap all NO_DANGLING reference values
existing *in* 'object' .
* If "removeRefsToObject" parameter is set to true, then *also* delete
from sReverseRefObjectMMap all
* NO_DANGLING references pointing *to* 'object.' .
* "removeRefsToObject" parameter is set to "false" by default, and is
an optional parameter.
*/
----------------------------------------------------
13) ImmModel::addNoDanglingReferenceSet(ObjectInfo *obj, ObjectNameSet
&dnSet)
The function adds references to sReverseRefObjectMap.
The leading comments could explain what references are constructed from the
input arguments. In essence each reference is constructed from one
element in the
dnSet argument as *source* and with obj as *target*.
But then it also filters on CREATE_LOCK on source/dnSet-element
<------------------------------
Something needs to be said about that ccb-state-relation.
Thus only references from sources that are currently objects being
created (not yet committed)
are inserted.
-------------------------------------------------------------------------------------------------------------
14) ImmModel::removeNoDanglingReferenceSet(ObjectInfo *obj,
ObjectNameSet &dnSet)
Again, comment describing the function does not say much.
Here is my understanding:
The function removes references from sReverseRefObjectMap.
For each dn in dnSet, it removes at most ONE reference in
sReverseRefObjectMap,
where the DN is the *target* and obj is the *source*
Contrary (unsymmetrically) to the addNoDanglingReferenceSet, this delete
function is not sensitive to
any CCB locking state.
--------------------------
15) In ImmModel::commitCreate(ObjectInfo* obj)
if(obj->mObjFlags & IMM_NO_DANGLING_FLAG) {
/* First remove all created NO_DANGLING references in the
operation phase,
* than add all NO_DANGLING references.
* Check for existence of objects is done before calling
ccbCommit function. */
removeNoDanglingReferences(obj, obj);
addNoDanglingReferences(obj);
}
The comment simply states what is obviously done in the next two statements.
It should explain *why* it is doing the remove.. before the create...,
which is not obvious.
-----------------------------------------------------------------------------------------------
16) In ImmModel::commitModify()
This is where the add/remove NoDanglingReferenceSet are used.
The comment:
+ /* Computation for which NO_DANGLING references will be removed,
and which will be added */
Should say something like:
The set of references in the before-image minus the set of references
in the after-image constitutes the
set of references *removed* by the (possibly chained) modify operation.
The set of references in the after-image minus the set of references in
the before-image constitutes the
set of references *added* by the (posibly chained) modify operation.
---------------------------------------------------------------------------------
17) In mmModel::commitDelete(const std::string& dn)
Unnecessary removal of reset of flag IMM_DELETE_ROOT.
A statement is removed:
if(oi->second->mObjFlags & IMM_DELETE_ROOT) {
oi->second->mObjFlags &= ~IMM_DELETE_ROOT; <===============
While not logically wrong, since the object is about to be deleted,
I dont like to see "cleanup" in logic that has nothing to do with
NO_DANGLING.
Also the resetting of the delete-root flag is in the same spirit as
zeroing members
in a struct/class before a delete/free. Its not necessary, but its a low
cost precaution,
(reduction of risk) in case an illegal reference remains somewhere
(i.e. a dangling hard pointer ... a bit OFF recursive irony).
In this case I am paranoid about getting that delete root bit wrong
anywhere because it affects the maintentance of the child-counter in the
imm tree,
which affects the correctness of all searches in the imm.
Imagine if someone rearranges this code in the future...
---------------------------------------------------------
Not quite done yet.
But getting close.
/AndersBj
------------------------------------------------------------------------------
Sponsored by Intel(R) XDK
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
