On Fri, 11 Jul 2025 at 18:04, Markus Armbruster <arm...@redhat.com> wrote:
>
> Peter Maydell <peter.mayd...@linaro.org> writes:
>
> > Coverity points out that the ivshmem-pci code has some error handling
> > cases where it incorrectly tries to use an invalid filedescriptor.
> > These generally happen because ivshmem_recv_msg() calls
> > qemu_chr_fe_get_msgfd(), which might return -1, but the code in
> > process_msg() generally assumes that the file descriptor was provided
> > when it was supposed to be. In particular:
> > * the error case in process_msg() only needs to close the fd
> > if one was provided
> > * process_msg_shmem() should fail if no fd was provided
> > if (msg < -1 || msg > IVSHMEM_MAX_PEERS) {
> > error_setg(errp, "server sent invalid message %" PRId64, msg);
> > - close(fd);
> > + if (fd >= 0) {
> > + close(fd);
> > + }
>
> Coverity is overly picky. close(-1) is *fine*. Just like free(NULL).
I think there is a distinction here. free(NULL) is documented
to be a correct program action which must have no effect.
close() on a bad file descriptor is documented to be an
error, which the implementation is supposed to diagnose.
Also, Coverity flags this kind of thing up because it tends to
indicate that whoever wrote the code was not actually thinking
about the error condition; so when it gets caught later this
is more by luck than judgement.
thanks
-- PMM
