Yongji Xie <xieyon...@bytedance.com> writes:
> On Wed, Jun 29, 2022 at 7:39 PM Markus Armbruster <arm...@redhat.com> wrote:
>>
>> Yongji Xie <xieyon...@bytedance.com> writes:
>>
>> > On Wed, Jun 29, 2022 at 5:41 PM Markus Armbruster <arm...@redhat.com>
>> > wrote:
>> >>
>> >> Xie Yongji <xieyon...@bytedance.com> writes:
>> >>
>> >> > Coverity pointed out (CID 1490222, 1490227) that we called
>> >> > ioctl somewhere without checking the return value. This
>> >> > patch fixes these issues.
>> >> >
>> >> > Fixes: Coverity CID 1490222, 1490227
>> >> > Signed-off-by: Xie Yongji <xieyon...@bytedance.com>
>> >> > ---
>> >> > subprojects/libvduse/libvduse.c | 10 ++++++++--
>> >> > 1 file changed, 8 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/subprojects/libvduse/libvduse.c
>> >> > b/subprojects/libvduse/libvduse.c
>> >> > index 1a5981445c..bf7302c60a 100644
>> >> > --- a/subprojects/libvduse/libvduse.c
>> >> > +++ b/subprojects/libvduse/libvduse.c
>> >> > @@ -947,7 +947,10 @@ static void vduse_queue_disable(VduseVirtq *vq)
>> >> >
>> >> > eventfd.index = vq->index;
>> >> > eventfd.fd = VDUSE_EVENTFD_DEASSIGN;
>> >> > - ioctl(dev->fd, VDUSE_VQ_SETUP_KICKFD, &eventfd);
>> >> > + if (ioctl(dev->fd, VDUSE_VQ_SETUP_KICKFD, &eventfd)) {
>> >> > + fprintf(stderr, "Failed to disable eventfd for vq[%d]: %s\n",
>> >> > + vq->index, strerror(errno));
>> >> > + }
>> >> > close(vq->fd);
>> >> >
>> >> > assert(vq->inuse == 0);
>> >> > @@ -1337,7 +1340,10 @@ VduseDev *vduse_dev_create(const char *name,
>> >> > uint32_t device_id,
>> >> >
>> >> > return dev;
>> >> > err:
>> >> > - ioctl(ctrl_fd, VDUSE_DESTROY_DEV, name);
>> >> > + if (ioctl(ctrl_fd, VDUSE_DESTROY_DEV, name)) {
>> >> > + fprintf(stderr, "Failed to destroy vduse device %s: %s\n",
>> >> > + name, strerror(errno));
>> >> > + }
>> >> > err_dev:
>> >> > close(ctrl_fd);
>> >> > err_ctrl:
>> >>
>> >> Both errors are during cleanup that can't fail. The program continues
>> >> as if they didn't happen. Does the user need to know?
>> >>
>> >
>> > So I printed some error messages. I didn't find any other good way to
>> > notify the users.
>>
>> I can think of another way, either. But my question wasn't about "how",
>> it was about "why". The answer depends on the impact of these errors.
>> Which I can't judge. Can you?
>>
>
> OK, I get your point. Actually users might have no way to handle those
> errors. And there is no real impact on users since those errors mean
> the resources have been cleaned up in other places or by other
> processes. So I choose to ignore this error, but it triggers a
> Coverity warning.
If we genuinely want to ignore errors from ioctl(), we can mark the
Coverity complaint as false positive.
