[Asterisk-code-review] res/res corosync: Raise a Stasis message on node join/leave ... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: res/res_corosync: Raise a Stasis message on node join/leave events
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/#/c/3109/1/res/res_corosync.c
File res/res_corosync.c:

Line 86: 	node = ao2_alloc(sizeof(*node), NULL);
You don't appear to need the lock on this ao2 object.  It does cost memory and time to have the lock created when not used.


PS1, Line 346: 	node = ao2_find(nodes, &id, OBJ_SEARCH_KEY);
             : 	if (node) {
             : 		/* We already know about this node */
             : 		ao2_ref(node, -1);
             : 		return;
             : 	}
             : 
             : 	node = corosync_node_alloc(event);
             : 	if (!node) {
             : 		return;
             : 	}
             : 	ao2_link(nodes, node);
             : 	ao2_ref(node, -1);
I think you need to have the nodes container lock protect the create if doesn't exist in the container operation.  Avoid the test and set race condition.


Line 594: 		node = ao2_find(nodes, &cpg_node->nodeid, OBJ_UNLINK | OBJ_SEARCH_KEY);
What if ao2_find doesn't find node?  You wind up with a NULL pointer dereference.


-- 
To view, visit https://gerrit.asterisk.org/3109
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9015f418d6ae7f47e4994e04e18948df4d49b465
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes