On Mon, 17 Mar 2025, Cédric Le Goater wrote:
On 3/14/25 21:01, BALATON Zoltan wrote:
Coverity reported that return value of blk_pwrite() maybe should not
be ignored. We can't do much if this happens other than report an
error but let's do that to silence this report.
Resolves: Coverity CID 1593725
Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
hw/ppc/amigaone.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 483512125f..5d787c3059 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -108,8 +108,8 @@ static void nvram_write(void *opaque, hwaddr addr,
uint64_t val,
uint8_t *p = memory_region_get_ram_ptr(&s->mr);
why is the nvram never read ?
There's a comment about that. It's a rom_device which maps the memory
region directly so does not go through the read callback. But I thin there
must be a read callback and cannot be null so we have an empty one.
Previously I had one that worked in case romd mode is turned off but Nick
said having dead code is not wanted and better to mark it unreachable.
p[addr] = val;
- if (s->blk) {
- blk_pwrite(s->blk, addr, 1, &val, 0);
+ if (s->blk && blk_pwrite(s->blk, addr, 1, &val, 0) < 0) {
+ error_report("%s: could not write %s", __func__,
blk_name(s->blk));
hmm, guest_error maybe ? since this is a runtime error.
It's not a guest error but some problem on the host with the backing file.
}
}
@@ -151,15 +151,17 @@ static void nvram_realize(DeviceState *dev, Error
**errp)
*c = cpu_to_be32(CRC32_DEFAULT_ENV);
/* Also copies terminating \0 as env is terminated by \0\0 */
memcpy(p + 4, default_env, sizeof(default_env));
- if (s->blk) {
- blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p,
0);
+ if (s->blk &&
+ blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p, 0)
< 0
+ ) {
+ error_report("%s: could not write %s", __func__,
blk_name(s->blk));
This should use the errp parameter.
}
return;
}
if (*c == 0) {
*c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
- if (s->blk) {
- blk_pwrite(s->blk, 0, 4, p, 0);
+ if (s->blk && blk_pwrite(s->blk, 0, 4, p, 0) < 0) {
+ error_report("%s: could not write %s", __func__,
blk_name(s->blk));
same here.
It could but I think it's not needed. It still works without the backing
file and the guest works, just may not save the NVRAM contents which is a
problem on the host. So the error is reported but I'm not sure it should
abort. In practice if there's some fatal error with the backing file the
blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
BLK_PERM_ALL, &error_fatal);
earlier will catch that so it won't even get here.
Regards,
BALATON Zoltan
