From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Tue, 9 Jun 2015 22:50:43 -0400 (EDT) Subject: [Cluster-devel] [PATCH] dlm: remove unnecessary error check In-Reply-To: <5577A36D.5010908@suse.com> References: <1433843172-8953-1-git-send-email-gqjiang@suse.com> <1545280635.13245793.1433851749238.JavaMail.zimbra@redhat.com> <5577A36D.5010908@suse.com> Message-ID: <70321158.13952725.1433904643714.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit ----- Original Message ----- > 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 Hi Guoqing, If remove_from_waiters can return -1, then the patch would prevent the code from calling remove_from_waiters. So the patch still doesn't look right to me. Regards, Bob Peterson Red Hat File Systems