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

[Matt Jordan]

Matt Jordan has posted comments on this change.

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


Patch Set 1:

(2 comments)

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

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 doe
You're technically correct, and I'll fix it to avoid said race.

However, if we had a race condition where two nodes existed in the same cluster with the same ID, there's going to be much bigger issues later on :-)


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 derefe
Egads! Fixed.


-- 
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: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes