From 4137577ae398837b0d5e47d4d9365320584efdad Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Tue, 5 Mar 2013 09:25:10 -0600 Subject: [PATCH] libceph: clean up skipped message logic In ceph_con_in_msg_alloc() it is possible for a connection's alloc_msg method to indicate an incoming message should be skipped. By default, read_partial_message() initializes the skip variable to 0 before it gets provided to ceph_con_in_msg_alloc(). The osd client, mon client, and mds client each supply an alloc_msg method. The mds client always assigns skip to be 0. The other two leave the skip value of as-is, or assigns it to zero, except: - if no (osd or mon) request having the given tid is found, in which case skip is set to 1 and NULL is returned; or - in the osd client, if the data of the reply message is not adequate to hold the message to be read, it assigns skip value 1 and returns NULL. So the returned message pointer will always be NULL if skip is ever non-zero. Clean up the logic a bit in ceph_con_in_msg_alloc() to make this state of affairs more obvious. Add a comment explaining how a null message pointer can mean either a message that should be skipped or a problem allocating a message. This resolves: http://tracker.ceph.com/issues/4324 Reported-by: Greg Farnum Signed-off-by: Alex Elder Reviewed-by: Greg Farnum --- net/ceph/messenger.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index c7d4278..af0c35d 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -2819,18 +2819,21 @@ static int ceph_con_in_msg_alloc(struct ceph_connection *con, int *skip) ceph_msg_put(msg); return -EAGAIN; } - con->in_msg = msg; - if (con->in_msg) { + if (msg) { + BUG_ON(*skip); + con->in_msg = msg; con->in_msg->con = con->ops->get(con); BUG_ON(con->in_msg->con == NULL); - } - if (*skip) { - con->in_msg = NULL; - return 0; - } - if (!con->in_msg) { - con->error_msg = - "error allocating memory for incoming message"; + } else { + /* + * Null message pointer means either we should skip + * this message or we couldn't allocate memory. The + * former is not an error. + */ + if (*skip) + return 0; + con->error_msg = "error allocating memory for incoming message"; + return -ENOMEM; } memcpy(&con->in_msg->hdr, &con->in_hdr, sizeof(con->in_hdr)); -- 2.7.4