Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754084AbbFJCmU (ORCPT ); Tue, 9 Jun 2015 22:42:20 -0400 Received: from smtp2.provo.novell.com ([137.65.250.81]:56347 "EHLO smtp2.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753910AbbFJCmN (ORCPT ); Tue, 9 Jun 2015 22:42:13 -0400 Message-ID: <5577A36D.5010908@suse.com> Date: Wed, 10 Jun 2015 10:39:41 +0800 From: Guoqing Jiang User-Agent: Thunderbird 2.0.0.24 (X11/20100302) MIME-Version: 1.0 To: Bob Peterson CC: ccaulfie@redhat.com, teigland@redhat.com, cluster-devel@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [Cluster-devel] [PATCH] dlm: remove unnecessary error check References: <1433843172-8953-1-git-send-email-gqjiang@suse.com> <1545280635.13245793.1433851749238.JavaMail.zimbra@redhat.com> In-Reply-To: <1545280635.13245793.1433851749238.JavaMail.zimbra@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1976 Lines: 69 Hi Bob, Bob Peterson wrote: > ----- Original Message ----- > >> We don't need the redundant logic since send_message always returns 0. >> >> Signed-off-by: Guoqing Jiang >> --- >> fs/dlm/lock.c | 10 ++-------- >> 1 file changed, 2 insertions(+), 8 deletions(-) >> >> diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c >> index 35502d4..6fc3de9 100644 >> --- a/fs/dlm/lock.c >> +++ b/fs/dlm/lock.c >> @@ -3656,10 +3656,7 @@ static int send_common(struct dlm_rsb *r, struct >> dlm_lkb *lkb, int mstype) >> >> send_args(r, lkb, ms); >> >> - error = send_message(mh, ms); >> - if (error) >> - goto fail; >> - return 0; >> + return send_message(mh, ms); >> >> fail: >> remove_from_waiters(lkb, msg_reply_type(mstype)); >> @@ -3763,10 +3760,7 @@ static int send_lookup(struct dlm_rsb *r, struct >> dlm_lkb *lkb) >> >> send_args(r, lkb, ms); >> >> - error = send_message(mh, ms); >> - if (error) >> - goto fail; >> - return 0; >> + return send_message(mh, ms); >> >> fail: >> remove_from_waiters(lkb, DLM_MSG_LOOKUP_REPLY); >> -- >> 1.7.12.4 >> > > Hi, > > The patch looks okay, but if remove_from_waiters() always returns 0, > wouldn't it be better to change the function from int to void and > return 0 here? The advantage is that code spelunkers wouldn't need > to back-track one more level (not to mention the instruction or two > it might save). > > Seems remove_from_waiters is not always returns 0, the return value could be -1 or 0 which depends on _remove_from_waiters. BTW, I found that there are no big difference between send_common and send_lookup, since the send_common can also be use to send lookup message, I guess send_lookup can be removed as well. Thanks, Guoqing -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/