[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/block: nvme: Fix a build error in nvme_process_sq()
From: |
Peter Maydell |
Subject: |
Re: [PATCH] hw/block: nvme: Fix a build error in nvme_process_sq() |
Date: |
Wed, 10 Feb 2021 11:01:12 +0000 |
On Wed, 10 Feb 2021 at 10:31, Klaus Jensen <its@irrelevant.dk> wrote:
> On Feb 10 18:24, Bin Meng wrote:
> > I am using the default GCC 5.4 on a Ubuntu 16.04 host.
> >
>
> Alright. I'm actually not sure why newer compilers does not report this.
> The warning looks reasonable.
It's not actually ever possible for nvme_ns() to return
NULL in this loop, because nvme_ns() will only return NULL
if it is passed an nsid value that is 0 or > n->num_namespaces,
and the loop conditions mean that we never do that. So
we can only end up using an uninitialized result if
n->num_namespaces is zero.
Newer compilers tend to do deeper analysis (eg inlining a
function like nvme_ns() here and analysing on the basis of
what that function does), so they can identify that
the "if (!ns) { continue; }" codepath is never taken.
I haven't actually done the analysis but I'm guessing that
newer compilers also manage to figure out somehow that it's not
possible to get here with n->num_namespaces being zero.
GCC 5.4 is not quite so sophisticated, so it can't tell.
There does seem to be a consistent pattern in the code of
for (i = 1; i <= n->num_namespaces; i++) {
ns = nvme_ns(n, i);
if (!ns) {
continue;
}
[stuff]
}
Might be worth considering replacing the "if (!ns) { continue; }"
with "assert(ns);".
thanks
-- PMM