| Message ID | 20240731143617.3391947-4-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Miscellaneous minor Coverity fixes | expand |
Am 31.07.2024 um 16:36 hat Peter Maydell geschrieben: > In pflash_write() Coverity points out that we can decrement the > unsigned pfl->counter below zero, which makes it wrap around. In > fact this is harmless, because if pfl->counter is 0 at this point we > also increment pfl->wcycle to 3, and the wcycle == 3 handling doesn't > look at counter; the only way back into code which looks at the > counter value is via wcycle == 1, which will reinitialize the counter. > But it's arguably a little clearer to break early in the "counter == > 0" if(), to avoid the decrement-below-zero. > > Resolves: Coverity CID 1547611 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 31/7/24 16:36, Peter Maydell wrote: > In pflash_write() Coverity points out that we can decrement the > unsigned pfl->counter below zero, which makes it wrap around. In > fact this is harmless, because if pfl->counter is 0 at this point we > also increment pfl->wcycle to 3, and the wcycle == 3 handling doesn't > look at counter; the only way back into code which looks at the > counter value is via wcycle == 1, which will reinitialize the counter. > But it's arguably a little clearer to break early in the "counter == > 0" if(), to avoid the decrement-below-zero. > > Resolves: Coverity CID 1547611 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/block/pflash_cfi01.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index c8f1cf5a872..2f3d1dd509c 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -614,6 +614,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset, if (!pfl->counter) { trace_pflash_write(pfl->name, "block write finished"); pfl->wcycle++; + break; } pfl->counter--;
In pflash_write() Coverity points out that we can decrement the unsigned pfl->counter below zero, which makes it wrap around. In fact this is harmless, because if pfl->counter is 0 at this point we also increment pfl->wcycle to 3, and the wcycle == 3 handling doesn't look at counter; the only way back into code which looks at the counter value is via wcycle == 1, which will reinitialize the counter. But it's arguably a little clearer to break early in the "counter == 0" if(), to avoid the decrement-below-zero. Resolves: Coverity CID 1547611 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/block/pflash_cfi01.c | 1 + 1 file changed, 1 insertion(+)