Fabio M. Di Nitto napsal(a):
> On 6/12/2013 5:03 PM, Jan Friesse wrote:
>> Signed-off-by: Jan Friesse <jfriesse@xxxxxxxxxx>
>> ---
>> exec/votequorum.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/exec/votequorum.c b/exec/votequorum.c
>> index b561409..131b734 100644
>> --- a/exec/votequorum.c
>> +++ b/exec/votequorum.c
>> @@ -1634,6 +1634,9 @@ static void message_handler_req_exec_votequorum_nodeinfo (
>>
>> if (nodeid == VOTEQUORUM_QDEVICE_NODEID) {
>> struct cluster_node *sender_node = find_node_by_nodeid(sender_nodeid);
>> +
>> + assert(sender_node != NULL);
>> +
>> if ((!cluster_is_quorate) &&
>> (sender_node->flags & NODE_FLAGS_QUORATE)) {
>> node->votes = req_exec_quorum_nodeinfo->votes;
>>
>
> I would prefer a proper error checking rather than assert here.
>
Agree. Actually, coverity found this problem, but I was ensure if it is
not problem (this is what I was assuming because of dereference of
sender_node) and then assert is proper solution OR it is problem and
then this patch starts discussion about proper solution.
> The logic is that a node always send its own information (to populate
> node for node == sender_nodeid) and then send quorum device info from
> that node.
>
> See static int votequorum_sync_process (void) that´s the one triggering
> the population of the node entries.
>
> In theory, if sync queues the messages as expected, sender_node is never
> NULL. If it is, then i´ll prefer at least to see a message that says:
> "received qdevice info before sender node info\n" and then quit (unless
> we can recover from that status easily).
>
Actually, because you've wrote that code you are maybe able to see how
to recover easily, but when I was reading that code, it really didn't
look so.
Honza
> Fabio
> _______________________________________________
> discuss mailing list
> discuss@xxxxxxxxxxxx
> http://lists.corosync.org/mailman/listinfo/discuss
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
