Re: [PATCH] hw/misc/sifive_u_otp: handling the fails of blk_pread and blk_pwrite

[Peter Maydell <peter.maydell@linaro.org>]

Ping! This patch was trying to fix a Coverity issue (CID 1435959,
1435960, 1435961) -- is anybody planning to review it?

(I'm not entirely sure 'guest error' is the right warning category,
but I don't know the specifics of this device.)

thanks
-- PMM

On Wed, 4 Nov 2020 at 09:29, Green Wan <green.wan@sifive.com> wrote:
>
> Fix code coverage issues by checking return value and handling fail case
> of blk_pread() and blk_pwrite(). Return default value 0xff if read fails.
>
> Signed-off-by: Green Wan <green.wan@sifive.com>
> ---
>  hw/misc/sifive_u_otp.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
> index 60066375ab..4314727d0d 100644
> --- a/hw/misc/sifive_u_otp.c
> +++ b/hw/misc/sifive_u_otp.c
> @@ -62,8 +62,13 @@ static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int size)
>              if (s->blk) {
>                  int32_t buf;
>
> -                blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf,
> -                          SIFIVE_U_OTP_FUSE_WORD);
> +                if (blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, &buf,
> +                              SIFIVE_U_OTP_FUSE_WORD) < 0) {
> +                    qemu_log_mask(LOG_GUEST_ERROR,
> +                                  "read error index<%d>\n", s->pa);
> +                    return 0xff;
> +                }
> +
>                  return buf;
>              }
>
> @@ -160,8 +165,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
>
>              /* write to backend */
>              if (s->blk) {
> -                blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD,
> -                           &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD, 0);
> +                if (blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD,
> +                               &s->fuse[s->pa], SIFIVE_U_OTP_FUSE_WORD,
> +                               0) < 0) {
> +                    qemu_log_mask(LOG_GUEST_ERROR,
> +                                  "write error index<%d>\n", s->pa);
> +                }
>              }
>
>              /* update written bit */
> @@ -248,12 +257,18 @@ static void sifive_u_otp_reset(DeviceState *dev)
>          int index = SIFIVE_U_OTP_SERIAL_ADDR;
>
>          serial_data = s->serial;
> -        blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> -                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +        if (blk_pwrite(s->blk, index * SIFIVE_U_OTP_FUSE_WORD,
> +                       &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0) < 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "write error index<%d>\n", index);
> +        }
>
>          serial_data = ~(s->serial);
> -        blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> -                   &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0);
> +        if (blk_pwrite(s->blk, (index + 1) * SIFIVE_U_OTP_FUSE_WORD,
> +                       &serial_data, SIFIVE_U_OTP_FUSE_WORD, 0) < 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "write error index<%d>\n", index + 1);
> +        }
>      }
>
>      /* Initialize write-once map */
> --
> 2.17.1