* [PATCH] afs: Fix logically dead code in afs_update_cell
@ 2019-05-27 16:54 Gustavo A. R. Silva
2019-05-28 15:42 ` Marc Dionne
2019-05-28 16:11 ` David Howells
0 siblings, 2 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2019-05-27 16:54 UTC (permalink / raw)
To: David Howells; +Cc: linux-afs, linux-kernel, Gustavo A. R. Silva
Fix logically dead code in switch statement.
Notice that *ret* is updated with -ENOMEM before the switch statement
at 395:
395 switch (ret) {
396 case -ENODATA:
397 case -EDESTADDRREQ:
398 vllist->status = DNS_LOOKUP_GOT_NOT_FOUND;
399 break;
400 case -EAGAIN:
401 case -ECONNREFUSED:
402 vllist->status = DNS_LOOKUP_GOT_TEMP_FAILURE;
403 break;
404 default:
405 vllist->status = DNS_LOOKUP_GOT_LOCAL_FAILURE;
406 break;
407 }
hence, the code in the switch (except for the default case) makes
no sense and is logically dead.
Fix this by removing the *ret* assignment at 390:
390 ret = -ENOMEM;
which is apparently wrong.
Addresses-Coverity-ID: 1445439 ("Logically dead code")
Fixes: d5c32c89b208 ("afs: Fix cell DNS lookup")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
fs/afs/cell.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/afs/cell.c b/fs/afs/cell.c
index 9c3b07ba2222..980de60bf060 100644
--- a/fs/afs/cell.c
+++ b/fs/afs/cell.c
@@ -387,7 +387,6 @@ static int afs_update_cell(struct afs_cell *cell)
if (ret == -ENOMEM)
goto out_wake;
- ret = -ENOMEM;
vllist = afs_alloc_vlserver_list(0);
if (!vllist)
goto out_wake;
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] afs: Fix logically dead code in afs_update_cell
2019-05-27 16:54 [PATCH] afs: Fix logically dead code in afs_update_cell Gustavo A. R. Silva
@ 2019-05-28 15:42 ` Marc Dionne
2019-05-28 16:11 ` David Howells
1 sibling, 0 replies; 3+ messages in thread
From: Marc Dionne @ 2019-05-28 15:42 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: David Howells, linux-afs, Linux Kernel Mailing List
On Mon, May 27, 2019 at 1:54 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
> Fix logically dead code in switch statement.
>
> Notice that *ret* is updated with -ENOMEM before the switch statement
> at 395:
>
> 395 switch (ret) {
> 396 case -ENODATA:
> 397 case -EDESTADDRREQ:
> 398 vllist->status = DNS_LOOKUP_GOT_NOT_FOUND;
> 399 break;
> 400 case -EAGAIN:
> 401 case -ECONNREFUSED:
> 402 vllist->status = DNS_LOOKUP_GOT_TEMP_FAILURE;
> 403 break;
> 404 default:
> 405 vllist->status = DNS_LOOKUP_GOT_LOCAL_FAILURE;
> 406 break;
> 407 }
>
> hence, the code in the switch (except for the default case) makes
> no sense and is logically dead.
>
> Fix this by removing the *ret* assignment at 390:
>
> 390 ret = -ENOMEM;
>
> which is apparently wrong.
>
> Addresses-Coverity-ID: 1445439 ("Logically dead code")
> Fixes: d5c32c89b208 ("afs: Fix cell DNS lookup")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> fs/afs/cell.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/afs/cell.c b/fs/afs/cell.c
> index 9c3b07ba2222..980de60bf060 100644
> --- a/fs/afs/cell.c
> +++ b/fs/afs/cell.c
> @@ -387,7 +387,6 @@ static int afs_update_cell(struct afs_cell *cell)
> if (ret == -ENOMEM)
> goto out_wake;
>
> - ret = -ENOMEM;
> vllist = afs_alloc_vlserver_list(0);
> if (!vllist)
> goto out_wake;
Looks like the intention here was to return -ENOMEM when
afs_alloc_vlserver_list fails, which would mean that the fix should
move the assignment within if (!vllist), rather than just removing it.
Although it might be fine to just return the error that came from
afs_dns_query instead, as you do in this patch.
Marc
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] afs: Fix logically dead code in afs_update_cell
2019-05-27 16:54 [PATCH] afs: Fix logically dead code in afs_update_cell Gustavo A. R. Silva
2019-05-28 15:42 ` Marc Dionne
@ 2019-05-28 16:11 ` David Howells
1 sibling, 0 replies; 3+ messages in thread
From: David Howells @ 2019-05-28 16:11 UTC (permalink / raw)
To: Marc Dionne
Cc: dhowells, Gustavo A. R. Silva, linux-afs, Linux Kernel Mailing List
Marc Dionne <marc.c.dionne@gmail.com> wrote:
> > diff --git a/fs/afs/cell.c b/fs/afs/cell.c
> > index 9c3b07ba2222..980de60bf060 100644
> > --- a/fs/afs/cell.c
> > +++ b/fs/afs/cell.c
> > @@ -387,7 +387,6 @@ static int afs_update_cell(struct afs_cell *cell)
> > if (ret == -ENOMEM)
> > goto out_wake;
> >
> > - ret = -ENOMEM;
> > vllist = afs_alloc_vlserver_list(0);
> > if (!vllist)
> > goto out_wake;
>
> Looks like the intention here was to return -ENOMEM when
> afs_alloc_vlserver_list fails, which would mean that the fix should
> move the assignment within if (!vllist), rather than just removing it.
> Although it might be fine to just return the error that came from
> afs_dns_query instead, as you do in this patch.
I think I'd rather return the original error as this patch effects. I'm
having a ponder on it.
David
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-28 16:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-27 16:54 [PATCH] afs: Fix logically dead code in afs_update_cell Gustavo A. R. Silva
2019-05-28 15:42 ` Marc Dionne
2019-05-28 16:11 ` David Howells
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.