Message ID | 20200414133052.13712-11-philmd@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | various bugfixes | expand |
Thank you for fixing the patch, Philippe! On Tue, Apr 14, 2020 at 9:31 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > From: Mansour Ahmadi <mansourweb@gmail.com> > > When updating the PFLASH file contents, we should check for a > possible failure of blk_pwrite(). Similar to commit 3a688294e. > > Signed-off-by: Mansour Ahmadi <mansourweb@gmail.com> > Message-Id: <20200408003552.58095-1-mansourweb@gmail.com> > [PMD: Add missing "qemu/error-report.h" include and TODO comment] > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/block/pflash_cfi01.c | 8 +++++++- > hw/block/pflash_cfi02.c | 8 +++++++- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c > index 24f3bce7ef..be1954c5d8 100644 > --- a/hw/block/pflash_cfi01.c > +++ b/hw/block/pflash_cfi01.c > @@ -42,6 +42,7 @@ > #include "hw/qdev-properties.h" > #include "sysemu/block-backend.h" > #include "qapi/error.h" > +#include "qemu/error-report.h" > #include "qemu/timer.h" > #include "qemu/bitops.h" > #include "qemu/error-report.h" > @@ -399,13 +400,18 @@ static void pflash_update(PFlashCFI01 *pfl, int > offset, > int size) > { > int offset_end; > + int ret; > if (pfl->blk) { > offset_end = offset + size; > /* widen to sector boundaries */ > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); > - blk_pwrite(pfl->blk, offset, pfl->storage + offset, > + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset, > offset_end - offset, 0); > + if (ret < 0) { > + /* TODO set error bit in status */ > + error_report("Could not update PFLASH: %s", strerror(-ret)); > + } > } > } > > diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c > index 12f18d401a..c6b6f2d082 100644 > --- a/hw/block/pflash_cfi02.c > +++ b/hw/block/pflash_cfi02.c > @@ -37,6 +37,7 @@ > #include "hw/block/flash.h" > #include "hw/qdev-properties.h" > #include "qapi/error.h" > +#include "qemu/error-report.h" > #include "qemu/bitmap.h" > #include "qemu/timer.h" > #include "sysemu/block-backend.h" > @@ -393,13 +394,18 @@ static uint64_t pflash_read(void *opaque, hwaddr > offset, unsigned int width) > static void pflash_update(PFlashCFI02 *pfl, int offset, int size) > { > int offset_end; > + int ret; > if (pfl->blk) { > offset_end = offset + size; > /* widen to sector boundaries */ > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); > - blk_pwrite(pfl->blk, offset, pfl->storage + offset, > + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset, > offset_end - offset, 0); > + if (ret < 0) { > + /* TODO set error bit in status */ > + error_report("Could not update PFLASH: %s", strerror(-ret)); > + } > } > } > > -- > 2.21.1 > >
On 4/14/20 3:30 PM, Philippe Mathieu-Daudé wrote: > From: Mansour Ahmadi <mansourweb@gmail.com> > > When updating the PFLASH file contents, we should check for a > possible failure of blk_pwrite(). Similar to commit 3a688294e. > There is actually a Coverity report for this issue, CID 1357678 (Unchecked return value) from 2016-07-15 06:28:48: CID 1357678 (#2 of 2): Unchecked return value (CHECKED_RETURN). check_return: Calling blk_pwrite without checking return value (as is done elsewhere 52 out of 59 times). So it seems fair to add: Reported-by: Coverity (CID 1357678 CHECKED_RETURN) > Signed-off-by: Mansour Ahmadi <mansourweb@gmail.com> > Message-Id: <20200408003552.58095-1-mansourweb@gmail.com> > [PMD: Add missing "qemu/error-report.h" include and TODO comment] > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/block/pflash_cfi01.c | 8 +++++++- > hw/block/pflash_cfi02.c | 8 +++++++- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c > index 24f3bce7ef..be1954c5d8 100644 > --- a/hw/block/pflash_cfi01.c > +++ b/hw/block/pflash_cfi01.c > @@ -42,6 +42,7 @@ > #include "hw/qdev-properties.h" > #include "sysemu/block-backend.h" > #include "qapi/error.h" > +#include "qemu/error-report.h" > #include "qemu/timer.h" > #include "qemu/bitops.h" > #include "qemu/error-report.h" > @@ -399,13 +400,18 @@ static void pflash_update(PFlashCFI01 *pfl, int offset, > int size) > { > int offset_end; > + int ret; > if (pfl->blk) { > offset_end = offset + size; > /* widen to sector boundaries */ > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); > - blk_pwrite(pfl->blk, offset, pfl->storage + offset, > + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset, > offset_end - offset, 0); > + if (ret < 0) { > + /* TODO set error bit in status */ > + error_report("Could not update PFLASH: %s", strerror(-ret)); > + } > } > } > > diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c > index 12f18d401a..c6b6f2d082 100644 > --- a/hw/block/pflash_cfi02.c > +++ b/hw/block/pflash_cfi02.c > @@ -37,6 +37,7 @@ > #include "hw/block/flash.h" > #include "hw/qdev-properties.h" > #include "qapi/error.h" > +#include "qemu/error-report.h" > #include "qemu/bitmap.h" > #include "qemu/timer.h" > #include "sysemu/block-backend.h" > @@ -393,13 +394,18 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width) > static void pflash_update(PFlashCFI02 *pfl, int offset, int size) > { > int offset_end; > + int ret; > if (pfl->blk) { > offset_end = offset + size; > /* widen to sector boundaries */ > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); > - blk_pwrite(pfl->blk, offset, pfl->storage + offset, > + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset, > offset_end - offset, 0); > + if (ret < 0) { > + /* TODO set error bit in status */ > + error_report("Could not update PFLASH: %s", strerror(-ret)); > + } > } > } > >
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 24f3bce7ef..be1954c5d8 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -42,6 +42,7 @@ #include "hw/qdev-properties.h" #include "sysemu/block-backend.h" #include "qapi/error.h" +#include "qemu/error-report.h" #include "qemu/timer.h" #include "qemu/bitops.h" #include "qemu/error-report.h" @@ -399,13 +400,18 @@ static void pflash_update(PFlashCFI01 *pfl, int offset, int size) { int offset_end; + int ret; if (pfl->blk) { offset_end = offset + size; /* widen to sector boundaries */ offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); - blk_pwrite(pfl->blk, offset, pfl->storage + offset, + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset, offset_end - offset, 0); + if (ret < 0) { + /* TODO set error bit in status */ + error_report("Could not update PFLASH: %s", strerror(-ret)); + } } } diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index 12f18d401a..c6b6f2d082 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -37,6 +37,7 @@ #include "hw/block/flash.h" #include "hw/qdev-properties.h" #include "qapi/error.h" +#include "qemu/error-report.h" #include "qemu/bitmap.h" #include "qemu/timer.h" #include "sysemu/block-backend.h" @@ -393,13 +394,18 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width) static void pflash_update(PFlashCFI02 *pfl, int offset, int size) { int offset_end; + int ret; if (pfl->blk) { offset_end = offset + size; /* widen to sector boundaries */ offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE); offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE); - blk_pwrite(pfl->blk, offset, pfl->storage + offset, + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset, offset_end - offset, 0); + if (ret < 0) { + /* TODO set error bit in status */ + error_report("Could not update PFLASH: %s", strerror(-ret)); + } } }