Re: [PATCH 2/3] hw/block/nvme: fix potential compilation error

[Minwoo Im]

On 21-02-22 08:06:14, Klaus Jensen wrote:
> From: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> 
> assert may be compiled to a noop and we could end up returning an
> uninitialized status.
> 
> Fix this by always returning Internal Device Error as a fallback.
> 
> Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/block/nvme.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index ddc83f7f7a19..897b9ff0db91 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1232,7 +1232,7 @@ static uint16_t nvme_check_zone_write(NvmeNamespace 
> *ns, NvmeZone *zone,
>  
>  static uint16_t nvme_check_zone_state_for_read(NvmeZone *zone)
>  {
> -    uint16_t status;
> +    uint64_t zslba = zone->d.zslba;
>  
>      switch (nvme_get_zone_state(zone)) {
>      case NVME_ZONE_STATE_EMPTY:
> @@ -1241,16 +1241,15 @@ static uint16_t 
> nvme_check_zone_state_for_read(NvmeZone *zone)
>      case NVME_ZONE_STATE_FULL:
>      case NVME_ZONE_STATE_CLOSED:
>      case NVME_ZONE_STATE_READ_ONLY:
> -        status = NVME_SUCCESS;
> -        break;
> +        return NVME_SUCCESS;
>      case NVME_ZONE_STATE_OFFLINE:
> -        status = NVME_ZONE_OFFLINE;
> -        break;
> +        trace_pci_nvme_err_zone_is_offline(zslba);

This also is a tiny addition to the potential error fix.  Anyway, it can
be shorten to: (if zslba is used in a place only)

        trace_pci_nvme_err_zone_is_offline(zone->d.zslba);

> +        return NVME_ZONE_OFFLINE;
>      default:
>          assert(false);
>      }
>  
> -    return status;
> +    return NVME_INTERNAL_DEV_ERROR;
>  }
>  
>  static uint16_t nvme_check_zone_read(NvmeNamespace *ns, uint64_t slba,
> -- 
> 2.30.1
> 
>