Hi On Thu, May 30, 2019 at 1:17 PM Peter Maydell <peter.mayd...@linaro.org> wrote: > > On Wed, 22 May 2019 at 09:29, Gerd Hoffmann <kra...@redhat.com> wrote: > > > > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > > > Add a vhost-user input backend example, based on virtio-input-host > > device. It takes an evdev path as argument, and can be associated with > > a vhost-user-input device via a UNIX socket: > > > > $ vhost-user-input -p /dev/input/eventX -s /tmp/vui.sock > > > > $ qemu ... -chardev socket,id=vuic,path=/tmp/vui.sock > > -device vhost-user-input-pci,chardev=vuic > > > > This example is intentionally not included in $TOOLS, and not > > installed by default. > > > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > > Message-id: 20190514104126.6294-4-marcandre.lur...@redhat.com > > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > > Hi; Coverity spotted a problem with this patch: > > > +int > > +main(int argc, char *argv[]) > > +{ > > + GMainLoop *loop = NULL; > > + VuInput vi = { 0, }; > > + int rc, ver, fd; > > + virtio_input_config id; > > + struct input_id ids; > > + GError *error = NULL; > > + GOptionContext *context; > > + > > + context = g_option_context_new(NULL); > > + g_option_context_add_main_entries(context, entries, NULL); > > + if (!g_option_context_parse(context, &argc, &argv, &error)) { > > + g_printerr("Option parsing failed: %s\n", error->message); > > + exit(EXIT_FAILURE); > > + } > > + if (opt_print_caps) { > > + g_print("{\n"); > > + g_print(" \"type\": \"input\",\n"); > > + g_print(" \"features\": [\n"); > > + g_print(" \"evdev-path\",\n"); > > + g_print(" \"no-grab\"\n"); > > + g_print(" ]\n"); > > + g_print("}\n"); > > + exit(EXIT_SUCCESS); > > + } > > + if (!opt_evdev) { > > + g_printerr("Please specify an evdev path\n"); > > + exit(EXIT_FAILURE); > > + } > > + if ((!!opt_socket_path + (opt_fdnum != -1)) != 1) { > > + g_printerr("Please specify either --fd or --socket-path\n"); > > + exit(EXIT_FAILURE); > > + } > > + > > + vi.evdevfd = open(opt_evdev, O_RDWR); > > + if (vi.evdevfd < 0) { > > + g_printerr("Failed to open evdev: %s\n", g_strerror(errno)); > > + exit(EXIT_FAILURE); > > + } > > + > > + rc = ioctl(vi.evdevfd, EVIOCGVERSION, &ver); > > + if (rc < 0) { > > + g_printerr("%s: is not an evdev device\n", argv[1]); > > + exit(EXIT_FAILURE); > > + } > > + > > + if (!opt_nograb) { > > + rc = ioctl(vi.evdevfd, EVIOCGRAB, 1); > > + if (rc < 0) { > > + g_printerr("Failed to grab device\n"); > > + exit(EXIT_FAILURE); > > + } > > + } > > + > > + vi.config = g_array_new(false, false, sizeof(virtio_input_config)); > > + memset(&id, 0, sizeof(id)); > > + ioctl(vi.evdevfd, EVIOCGNAME(sizeof(id.u.string) - 1), id.u.string); > > CID 1401704 -- we don't check the return value from ioctl().
ok, I'll send a fix > > > + id.select = VIRTIO_INPUT_CFG_ID_NAME; > > + id.size = strlen(id.u.string); > > + g_array_append_val(vi.config, id); > > + > > + if (ioctl(vi.evdevfd, EVIOCGID, &ids) == 0) { > > + memset(&id, 0, sizeof(id)); > > + id.select = VIRTIO_INPUT_CFG_ID_DEVIDS; > > + id.size = sizeof(struct virtio_input_devids); > > + id.u.ids.bustype = cpu_to_le16(ids.bustype); > > + id.u.ids.vendor = cpu_to_le16(ids.vendor); > > + id.u.ids.product = cpu_to_le16(ids.product); > > + id.u.ids.version = cpu_to_le16(ids.version); > > + g_array_append_val(vi.config, id); > > + } > > + > > + vi_bits_config(&vi, EV_KEY, KEY_CNT); > > + vi_bits_config(&vi, EV_REL, REL_CNT); > > + vi_bits_config(&vi, EV_ABS, ABS_CNT); > > + vi_bits_config(&vi, EV_MSC, MSC_CNT); > > + vi_bits_config(&vi, EV_SW, SW_CNT); > > + g_debug("config length: %u", vi.config->len); > > + > > + if (opt_socket_path) { > > + int lsock = unix_listen(opt_socket_path, &error_fatal); > > + fd = accept(lsock, NULL, NULL); > > + close(lsock); > > This is CID 1401705 -- failure to check return value from > unix_listen() -- which I just realised I probably replied > to the wrong version of the patch to point out, so I mention > it again here. coverity should realize that passing &error_fatal will not return, no? Can we mark this as false-positive? > > > + } else { > > + fd = opt_fdnum; > > + } > > thanks > -- PMM