Re: [PATCH] hw/block: nvme: Fix a build error in nvme_process_sq()

[Peter Maydell]

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