| Message ID | 20200226084647.20636-2-kuhn.chenqun@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | redundant code: Fix warnings reported by Clang static code analyzer | expand |
Am 26.02.2020 um 09:46 hat kuhn.chenqun@huawei.com geschrieben: > From: Chen Qun <kuhn.chenqun@huawei.com> > > Clang static code analyzer show warning: > block/stream.c:186:9: warning: Value stored to 'ret' is never read > ret = 0; > ^ ~ > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > Reviewed-by: John Snow <jsnow@redhat.com> Let's mention that this is unnecessary since commit 1d809098aa9. Since the same commit, the initialisation 'int ret = 0;' is unnecessary because we never read ret before overwriting the initial value. We could clean this up in the same patch. With or without the changes: Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>-----Original Message----- >From: Kevin Wolf [mailto:kwolf@redhat.com] >Sent: Wednesday, February 26, 2020 5:51 PM >To: Chenqun (kuhn) <kuhn.chenqun@huawei.com> >Cc: qemu-devel@nongnu.org; qemu-trivial@nongnu.org; >peter.maydell@linaro.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; >Euler Robot <euler.robot@huawei.com>; John Snow <jsnow@redhat.com>; >Max Reitz <mreitz@redhat.com> >Subject: Re: [PATCH v2 01/13] block/stream: Remove redundant statement in >stream_run() > >Am 26.02.2020 um 09:46 hat kuhn.chenqun@huawei.com geschrieben: >> From: Chen Qun <kuhn.chenqun@huawei.com> >> >> Clang static code analyzer show warning: >> block/stream.c:186:9: warning: Value stored to 'ret' is never read >> ret = 0; >> ^ ~ >> Reported-by: Euler Robot <euler.robot@huawei.com> >> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> >> Reviewed-by: John Snow <jsnow@redhat.com> > >Let's mention that this is unnecessary since commit 1d809098aa9. > >Since the same commit, the initialisation 'int ret = 0;' is unnecessary because >we never read ret before overwriting the initial value. We could clean this up >in the same patch. Yes, we can clean it and move 'ret' declaration to the for() statement. Modify just Like this： @@ -114,7 +114,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp) int64_t offset = 0; uint64_t delay_ns = 0; int error = 0; - int ret = 0; int64_t n = 0; /* bytes */ if (bs == s->bottom) { @@ -139,6 +138,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp) for ( ; offset < len; offset += n) { bool copy; + int ret; /* Note that even when no rate limit is applied we need to yield * with no pending I/O here so that bdrv_drain_all() returns. @@ -183,7 +183,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp) break; } } - ret = 0; /* Publish progress */ job_progress_update(&s->common.job, n); > >With or without the changes: > >Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Am 26.02.2020 um 11:21 hat Chenqun (kuhn) geschrieben: > > > >-----Original Message----- > >From: Kevin Wolf [mailto:kwolf@redhat.com] > >Sent: Wednesday, February 26, 2020 5:51 PM > >To: Chenqun (kuhn) <kuhn.chenqun@huawei.com> > >Cc: qemu-devel@nongnu.org; qemu-trivial@nongnu.org; > >peter.maydell@linaro.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; > >Euler Robot <euler.robot@huawei.com>; John Snow <jsnow@redhat.com>; > >Max Reitz <mreitz@redhat.com> > >Subject: Re: [PATCH v2 01/13] block/stream: Remove redundant statement in > >stream_run() > > > >Am 26.02.2020 um 09:46 hat kuhn.chenqun@huawei.com geschrieben: > >> From: Chen Qun <kuhn.chenqun@huawei.com> > >> > >> Clang static code analyzer show warning: > >> block/stream.c:186:9: warning: Value stored to 'ret' is never read > >> ret = 0; > >> ^ ~ > >> Reported-by: Euler Robot <euler.robot@huawei.com> > >> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > >> Reviewed-by: John Snow <jsnow@redhat.com> > > > >Let's mention that this is unnecessary since commit 1d809098aa9. > > > >Since the same commit, the initialisation 'int ret = 0;' is unnecessary because > >we never read ret before overwriting the initial value. We could clean this up > >in the same patch. > > Yes, we can clean it and move 'ret' declaration to the for() statement. > > Modify just Like this： > [...] Godd point, makes sense to me. Please keep my R-b if you make this change. Kevin > @@ -114,7 +114,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp) > int64_t offset = 0; > uint64_t delay_ns = 0; > int error = 0; > - int ret = 0; > int64_t n = 0; /* bytes */ > > if (bs == s->bottom) { > @@ -139,6 +138,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp) > > for ( ; offset < len; offset += n) { > bool copy; > + int ret; > > /* Note that even when no rate limit is applied we need to yield > * with no pending I/O here so that bdrv_drain_all() returns. > @@ -183,7 +183,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp) > break; > } > } > - ret = 0; > > /* Publish progress */ > job_progress_update(&s->common.job, n); > > > > >With or without the changes: > > > >Reviewed-by: Kevin Wolf <kwolf@redhat.com> >
>-----Original Message----- >From: Kevin Wolf [mailto:kwolf@redhat.com] >Sent: Wednesday, February 26, 2020 6:36 PM >To: Chenqun (kuhn) <kuhn.chenqun@huawei.com> >Cc: John Snow <jsnow@redhat.com>; qemu-devel@nongnu.org; qemu- >trivial@nongnu.org; peter.maydell@linaro.org; Zhanghailiang ><zhang.zhanghailiang@huawei.com>; Euler Robot ><euler.robot@huawei.com>; Max Reitz <mreitz@redhat.com> >Subject: Re: [PATCH v2 01/13] block/stream: Remove redundant statement in >stream_run() > >Am 26.02.2020 um 11:21 hat Chenqun (kuhn) geschrieben: >> >> >> >-----Original Message----- >> >From: Kevin Wolf [mailto:kwolf@redhat.com] >> >Sent: Wednesday, February 26, 2020 5:51 PM >> >To: Chenqun (kuhn) <kuhn.chenqun@huawei.com> >> >Cc: qemu-devel@nongnu.org; qemu-trivial@nongnu.org; >> >peter.maydell@linaro.org; Zhanghailiang >> ><zhang.zhanghailiang@huawei.com>; Euler Robot >> ><euler.robot@huawei.com>; John Snow <jsnow@redhat.com>; Max Reitz >> ><mreitz@redhat.com> >> >Subject: Re: [PATCH v2 01/13] block/stream: Remove redundant >> >statement in >> >stream_run() >> > >> >Am 26.02.2020 um 09:46 hat kuhn.chenqun@huawei.com geschrieben: >> >> From: Chen Qun <kuhn.chenqun@huawei.com> >> >> >> >> Clang static code analyzer show warning: >> >> block/stream.c:186:9: warning: Value stored to 'ret' is never read >> >> ret = 0; >> >> ^ ~ >> >> Reported-by: Euler Robot <euler.robot@huawei.com> >> >> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> >> >> Reviewed-by: John Snow <jsnow@redhat.com> >> > >> >Let's mention that this is unnecessary since commit 1d809098aa9. >> > >> >Since the same commit, the initialisation 'int ret = 0;' is >> >unnecessary because we never read ret before overwriting the initial >> >value. We could clean this up in the same patch. >> >> Yes, we can clean it and move 'ret' declaration to the for() statement. >> >> Modify just Like this： >> [...] > >Godd point, makes sense to me. Please keep my R-b if you make this change. OK, no problem！ Thanks. > >Kevin > >> @@ -114,7 +114,6 @@ static int coroutine_fn stream_run(Job *job, Error >**errp) >> int64_t offset = 0; >> uint64_t delay_ns = 0; >> int error = 0; >> - int ret = 0; >> int64_t n = 0; /* bytes */ >> >> if (bs == s->bottom) { >> @@ -139,6 +138,7 @@ static int coroutine_fn stream_run(Job *job, Error >> **errp) >> >> for ( ; offset < len; offset += n) { >> bool copy; >> + int ret; >> >> /* Note that even when no rate limit is applied we need to yield >> * with no pending I/O here so that bdrv_drain_all() returns. >> @@ -183,7 +183,6 @@ static int coroutine_fn stream_run(Job *job, Error >**errp) >> break; >> } >> } >> - ret = 0; >> >> /* Publish progress */ >> job_progress_update(&s->common.job, n); >> >> > >> >With or without the changes: >> > >> >Reviewed-by: Kevin Wolf <kwolf@redhat.com> >>
diff --git a/block/stream.c b/block/stream.c index 5562ccbf57..d78074ac80 100644 --- a/block/stream.c +++ b/block/stream.c @@ -183,7 +183,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp) break; } } - ret = 0; /* Publish progress */ job_progress_update(&s->common.job, n);