lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 10 Jun 2015 11:10:44 +0800 From: Guoqing Jiang <gqJiang@...e.com> To: Bob Peterson <rpeterso@...hat.com> CC: ccaulfie@...hat.com, teigland@...hat.com, cluster-devel@...hat.com, linux-kernel@...r.kernel.org Subject: Re: [Cluster-devel] [PATCH] dlm: remove unnecessary error check Bob Peterson wrote: > ----- 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 <gqjiang@...e.com> >>>> --- >>>> 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. > > Hi Bob, The remove_from_waiters could only be invoked after failed to create_message, right? Since send_message always returns 0, this patch doesn't touch anything about the failure path, and it also doesn't change the original semantic. Thanks, Guoqing -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists