| Message ID | 20200611153934.928021-1-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam/qi2: remove redundant assignment to ret | expand |
On Thu, Jun 11, 2020 at 04:39:34PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The variable ret is being assigned a value that is never read, the > error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning > ret with -ENOMEM is redundamt. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/crypto/caam/caamalg_qi2.c | 2 -- > 1 file changed, 2 deletions(-) Patch applied. Thanks.
On 6/18/2020 10:58 AM, Herbert Xu wrote: > On Thu, Jun 11, 2020 at 04:39:34PM +0100, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> The variable ret is being assigned a value that is never read, the >> error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning >> ret with -ENOMEM is redundamt. >> >> Addresses-Coverity: ("Unused value") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/crypto/caam/caamalg_qi2.c | 2 -- >> 1 file changed, 2 deletions(-) > > Patch applied. Thanks. > Unfortunately I missed this patch, and it doesn't look correct. Do I need to send a revert? Thanks, Horia
On Thu, Jun 18, 2020 at 01:40:55PM +0300, Horia Geantă wrote: > On 6/18/2020 10:58 AM, Herbert Xu wrote: > > On Thu, Jun 11, 2020 at 04:39:34PM +0100, Colin King wrote: > >> From: Colin Ian King <colin.king@canonical.com> > >> > >> The variable ret is being assigned a value that is never read, the > >> error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning > >> ret with -ENOMEM is redundamt. > >> > >> Addresses-Coverity: ("Unused value") > >> Signed-off-by: Colin Ian King <colin.king@canonical.com> > >> --- > >> drivers/crypto/caam/caamalg_qi2.c | 2 -- > >> 1 file changed, 2 deletions(-) > > > > Patch applied. Thanks. > > > Unfortunately I missed this patch, and it doesn't look correct. > > Do I need to send a revert? No please check again. The patch is correct. Thanks,
On 6/11/2020 6:39 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The variable ret is being assigned a value that is never read, the > error exit path via label 'unmap' returns -ENOMEM anyhow, so assigning > ret with -ENOMEM is redundamt. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > drivers/crypto/caam/caamalg_qi2.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c > index 28669cbecf77..ef2c4e095db3 100644 > --- a/drivers/crypto/caam/caamalg_qi2.c > +++ b/drivers/crypto/caam/caamalg_qi2.c > @@ -4044,7 +4044,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) > DMA_TO_DEVICE); > if (dma_mapping_error(ctx->dev, edesc->qm_sg_dma)) { > dev_err(ctx->dev, "unable to map S/G table\n"); > - ret = -ENOMEM; > goto unmap; > } > edesc->qm_sg_bytes = qm_sg_bytes; > @@ -4055,7 +4054,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) > if (dma_mapping_error(ctx->dev, state->ctx_dma)) { > dev_err(ctx->dev, "unable to map ctx\n"); > state->ctx_dma = 0; > - ret = -ENOMEM; > goto unmap; > } > The proper fix would be updating the ahash_finup_no_ctx() function to return the specific error code: return ret; instead of returning -ENOMEM for all error cases. For example error code returned by dpaa2_caam_enqueue() should be returned instead of -ENOMEM. Horia
On Thu, Jun 18, 2020 at 01:54:55PM +0300, Horia Geantă wrote: > > The proper fix would be updating the ahash_finup_no_ctx() function > to return the specific error code: > return ret; > instead of returning -ENOMEM for all error cases. > > For example error code returned by dpaa2_caam_enqueue() > should be returned instead of -ENOMEM. You can do that as a follow-up. The patch is correct as is. Cheers,
On 6/18/2020 2:00 PM, Herbert Xu wrote: > On Thu, Jun 18, 2020 at 01:54:55PM +0300, Horia Geantă wrote: >> >> The proper fix would be updating the ahash_finup_no_ctx() function >> to return the specific error code: >> return ret; >> instead of returning -ENOMEM for all error cases. >> >> For example error code returned by dpaa2_caam_enqueue() >> should be returned instead of -ENOMEM. > > You can do that as a follow-up. The patch is correct as is. > Just that the follow-up implies adding all the code back. Anyway, not a big deal... Thanks, Horia
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c index 28669cbecf77..ef2c4e095db3 100644 --- a/drivers/crypto/caam/caamalg_qi2.c +++ b/drivers/crypto/caam/caamalg_qi2.c @@ -4044,7 +4044,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) DMA_TO_DEVICE); if (dma_mapping_error(ctx->dev, edesc->qm_sg_dma)) { dev_err(ctx->dev, "unable to map S/G table\n"); - ret = -ENOMEM; goto unmap; } edesc->qm_sg_bytes = qm_sg_bytes; @@ -4055,7 +4054,6 @@ static int ahash_finup_no_ctx(struct ahash_request *req) if (dma_mapping_error(ctx->dev, state->ctx_dma)) { dev_err(ctx->dev, "unable to map ctx\n"); state->ctx_dma = 0; - ret = -ENOMEM; goto unmap; }