| Message ID | 1618391864-55601-1-git-send-email-tiantao6@hisilicon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: ccp - Fix to return the correct return value | expand |
On 4/14/21 4:17 AM, Tian Tao wrote: > ccp_dev_suspend and ccp_dev_resume return 0 on error, which causes > ret to equal 0 in sp_suspend and sp_resume, making the if condition > impossible to use. Why do you think that is an error and why do you think it should return -ENXIO? Since ccp_dev_suspend() and ccp_dev_resume() only return 0 it might be a more appropriate fix to have these be void functions and eliminate the if condition in sp_suspend() and sp_resume(). Thanks, Tom > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com> > --- > drivers/crypto/ccp/ccp-dev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c > index 0971ee6..6f2af7b 100644 > --- a/drivers/crypto/ccp/ccp-dev.c > +++ b/drivers/crypto/ccp/ccp-dev.c > @@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp) > > /* If there's no device there's nothing to do */ > if (!ccp) > - return 0; > + return -ENXIO; > > spin_lock_irqsave(&ccp->cmd_lock, flags); > > @@ -584,7 +584,7 @@ int ccp_dev_resume(struct sp_device *sp) > > /* If there's no device there's nothing to do */ > if (!ccp) > - return 0; > + return -ENXIO; > > spin_lock_irqsave(&ccp->cmd_lock, flags); > >
在 2021/4/15 6:48, Tom Lendacky 写道: > On 4/14/21 4:17 AM, Tian Tao wrote: >> ccp_dev_suspend and ccp_dev_resume return 0 on error, which causes >> ret to equal 0 in sp_suspend and sp_resume, making the if condition >> impossible to use. > Why do you think that is an error and why do you think it should return > -ENXIO? Since ccp_dev_suspend() and ccp_dev_resume() only return 0 it thank you for helping reivew. I think that ccp equals null might just be wrong, now after listening to your explanation, my understanding was wrong, I will send a new patch as you suggested. > might be a more appropriate fix to have these be void functions and > eliminate the if condition in sp_suspend() and sp_resume(). > > Thanks, > Tom > >> Signed-off-by: Tian Tao <tiantao6@hisilicon.com> >> --- >> drivers/crypto/ccp/ccp-dev.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c >> index 0971ee6..6f2af7b 100644 >> --- a/drivers/crypto/ccp/ccp-dev.c >> +++ b/drivers/crypto/ccp/ccp-dev.c >> @@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp) >> >> /* If there's no device there's nothing to do */ >> if (!ccp) >> - return 0; >> + return -ENXIO; >> >> spin_lock_irqsave(&ccp->cmd_lock, flags); >> >> @@ -584,7 +584,7 @@ int ccp_dev_resume(struct sp_device *sp) >> >> /* If there's no device there's nothing to do */ >> if (!ccp) >> - return 0; >> + return -ENXIO; >> >> spin_lock_irqsave(&ccp->cmd_lock, flags); >> >> > . >
diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c index 0971ee6..6f2af7b 100644 --- a/drivers/crypto/ccp/ccp-dev.c +++ b/drivers/crypto/ccp/ccp-dev.c @@ -556,7 +556,7 @@ int ccp_dev_suspend(struct sp_device *sp) /* If there's no device there's nothing to do */ if (!ccp) - return 0; + return -ENXIO; spin_lock_irqsave(&ccp->cmd_lock, flags); @@ -584,7 +584,7 @@ int ccp_dev_resume(struct sp_device *sp) /* If there's no device there's nothing to do */ if (!ccp) - return 0; + return -ENXIO; spin_lock_irqsave(&ccp->cmd_lock, flags);
ccp_dev_suspend and ccp_dev_resume return 0 on error, which causes ret to equal 0 in sp_suspend and sp_resume, making the if condition impossible to use. Signed-off-by: Tian Tao <tiantao6@hisilicon.com> --- drivers/crypto/ccp/ccp-dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)