I just noticed I forgot to cc Peter Lieven on this email -- sorry about that.
On Thu, 12 Mar 2026 at 16:47, Kevin Wolf <[email protected]> wrote: > > Am 12.03.2026 um 17:19 hat Peter Maydell geschrieben: > > On Thu, 12 Mar 2026 at 16:13, Kevin Wolf <[email protected]> wrote: > > > > > > Am 12.03.2026 um 10:41 hat Peter Maydell geschrieben: > > > > On Tue, 10 Mar 2026 at 16:30, Kevin Wolf <[email protected]> wrote: > > > > > > > > > > From: Peter Lieven <[email protected]> > > > > > > > > > > libnfs v6 added a new api structure for read and write requests. > > > > > > > > > > This effectively also adds zero copy read support for cases where > > > > > the qiov coming from the block layer has only one vector. > > > > > > > > > > The .brdv_refresh_limits implementation is needed because libnfs v6 > > > > > silently dropped support for splitting large read/write request into > > > > > chunks. > > > > > > > > > > Signed-off-by: Ronnie Sahlberg <[email protected]> > > > > > Signed-off-by: Peter Lieven <[email protected]> > > > > > Message-ID: <[email protected]> > > > > > Reviewed-by: Kevin Wolf <[email protected]> > > > > > Signed-off-by: Kevin Wolf <[email protected]> > > > > > > > > > > > > Hi; Coverity reports a potential issue with this code > > > > (CID 1645631): > > > > > + if (my_buffer) { > > > > > + if (task.ret > 0) { > > > > > + qemu_iovec_from_buf(iov, 0, buf, task.ret); > > > > > > > > ...and down here we use 'buf', but Coverity thinks it might be NULL > > > > because we only returned -ENOMEM above for a NULL buffer if bytes == 0. > > > > So we might be here with bytes == 0 and buf == NULL. > > > > > > Yes, buf might be NULL, but copying 0 bytes from NULL isn't a problem > > > because you don't actually dereference the pointer then. > > > > > > I think the part that Coverity doesn't understand is probably that > > > task.ret is limited to bytes (i.e. 0 in this case). > > > > Maybe we can't get here, but maybe it would be simpler to handle > > > > the "asked to read 0 bytes" case directly without calling into the > > > > nfs library or allocating a 0 byte buffer? > > > We could have an 'if (!bytes) { return 0; }' at the start of the > > > function if we want to. > > Mmm. Let me know if you'd prefer me to mark this issue in > > Coverity as a false positive rather than changing the code. > > I don't really mind either way. If someone posts a patch, I'll apply it > (though not sure if that would be only for 11.1 at this point). So what is the conclusion here -- do we want to change the code, or are we happy with it as it is and want to tell Coverity this is a false positive? thanks -- PMM
