> -----Original Message-----
> From: Keith Busch <kbu...@kernel.org>
> Sent: Tuesday, October 13, 2020 8:51 PM
> To: Dmitry Fomichev <dmitry.fomic...@wdc.com>
> Cc: Klaus Jensen <k.jen...@samsung.com>; Kevin Wolf
> <kw...@redhat.com>; Philippe Mathieu-Daudé <phi...@redhat.com>;
> Maxim Levitsky <mlevi...@redhat.com>; Fam Zheng <f...@euphon.net>;
> Alistair Francis <alistair.fran...@wdc.com>; Matias Bjorling
> <matias.bjorl...@wdc.com>; Niklas Cassel <niklas.cas...@wdc.com>;
> Damien Le Moal <damien.lem...@wdc.com>; qemu-block@nongnu.org;
> qemu-de...@nongnu.org
> Subject: Re: [PATCH v6 01/11] hw/block/nvme: Add Commands Supported
> and Effects log
> 
> On Wed, Oct 14, 2020 at 06:42:02AM +0900, Dmitry Fomichev wrote:
> > +{
> > +    NvmeEffectsLog log = {};
> > +    uint32_t *dst_acs = log.acs, *dst_iocs = log.iocs;
> > +    uint32_t trans_len;
> > +    int i;
> > +
> > +    trace_pci_nvme_cmd_supp_and_effects_log_read();
> > +
> > +    if (off >= sizeof(log)) {
> > +        trace_pci_nvme_err_invalid_effects_log_offset(off);
> > +        return NVME_INVALID_FIELD | NVME_DNR;
> > +    }
> > +
> > +    for (i = 0; i < 256; i++) {
> > +        dst_acs[i] = nvme_cse_acs[i];
> > +    }
> > +
> > +    for (i = 0; i < 256; i++) {
> > +        dst_iocs[i] = nvme_cse_iocs_nvm[i];
> > +    }
> 
> You're just copying the array, so let's do it like this:
> 
>     memcpy(log.acs, nvme_cse_acs, sizeof(nvme_cse_acs));
>     memcpy(log.iocs, nvme_cse_iocs_nvm, sizeof(nvme_cse_iocs_nvm));
> 

Sure, this is a lot cleaner.

> I think you also need to check
> 
>     if (NVME_CC_CSS(n->bar.cc) != NVME_CC_CSS_ADMIN_ONLY)
> 
> before copying iocs.

Yes, thanks.


Reply via email to