From 718fce997a0bdf8ad163402c2c35a8b1d6f0c0ac Mon Sep 17 00:00:00 2001 From: Franck LeCodeur Date: Tue, 12 Oct 2021 16:39:45 +0200 Subject: [PATCH] drivers/input: Fix warnings and enable Werror Correct signedness for comparisons, remove unused code, remove unused variable, correct variable type for callbacks Part of #9460 Change-Id: Ie48e8498e0830ed8b175986aaf82b94a1d99b72f Reviewed-on: https://review.haiku-os.org/c/haiku/+/4570 Tested-by: Commit checker robot Reviewed-by: Adrien Destugues --- build/jam/ArchitectureRules | 2 +- .../input/hid_shared/HIDCollection.cpp | 9 +- .../drivers/input/hid_shared/HIDReport.cpp | 3 +- .../input/virtio_input/virtio_input.cpp | 24 +-- .../kernel/drivers/input/wacom/wacom.c | 156 +++++++++--------- 5 files changed, 87 insertions(+), 107 deletions(-) diff --git a/build/jam/ArchitectureRules b/build/jam/ArchitectureRules index 0a0e28a527..d9785bf4df 100644 --- a/build/jam/ArchitectureRules +++ b/build/jam/ArchitectureRules @@ -667,7 +667,7 @@ rule ArchitectureSetupWarnings architecture EnableWerror src add-ons kernel drivers dvb ; # EnableWerror src add-ons kernel drivers graphics ; EnableWerror src add-ons kernel drivers graphics intel_extreme ; -# EnableWerror src add-ons kernel drivers input ; + EnableWerror src add-ons kernel drivers input ; EnableWerror src add-ons kernel drivers joystick ; EnableWerror src add-ons kernel drivers midi ; EnableWerror src add-ons kernel drivers misc ; diff --git a/src/add-ons/kernel/drivers/input/hid_shared/HIDCollection.cpp b/src/add-ons/kernel/drivers/input/hid_shared/HIDCollection.cpp index f466cadd37..58d8de37ff 100644 --- a/src/add-ons/kernel/drivers/input/hid_shared/HIDCollection.cpp +++ b/src/add-ons/kernel/drivers/input/hid_shared/HIDCollection.cpp @@ -83,7 +83,8 @@ HIDCollection::AddChild(HIDCollection *child) HIDCollection * HIDCollection::ChildAt(uint32 index) { - if (index >= fChildren.Count()) + int32 count = fChildren.Count(); + if (count < 0 || index >= (uint32)count) return NULL; return fChildren[index]; @@ -129,7 +130,8 @@ HIDCollection::AddItem(HIDReportItem *item) HIDReportItem * HIDCollection::ItemAt(uint32 index) { - if (index >= fItems.Count()) + int32 count = fItems.Count(); + if (count < 0 || index >= (uint32)count) return NULL; return fItems[index]; @@ -239,7 +241,8 @@ HIDCollection::_ChildAtFlat(uint8 type, uint32 &index) HIDReportItem * HIDCollection::_ItemAtFlat(uint32 &index) { - if (index < fItems.Count()) + int32 count = fItems.Count(); + if (count > 0 && index < (uint32)count) return fItems[index]; index -= fItems.Count(); diff --git a/src/add-ons/kernel/drivers/input/hid_shared/HIDReport.cpp b/src/add-ons/kernel/drivers/input/hid_shared/HIDReport.cpp index 7cad4114c6..1687c53c4f 100644 --- a/src/add-ons/kernel/drivers/input/hid_shared/HIDReport.cpp +++ b/src/add-ons/kernel/drivers/input/hid_shared/HIDReport.cpp @@ -194,7 +194,8 @@ HIDReport::SendReport() HIDReportItem * HIDReport::ItemAt(uint32 index) { - if (index >= fItems.Count()) + int32 count = fItems.Count(); + if (count < 0 || index >= (uint32)count) return NULL; return fItems[index]; } diff --git a/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp b/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp index e554ca1f2c..53b1b55583 100644 --- a/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp +++ b/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp @@ -71,7 +71,7 @@ struct VirtioInputHandle { device_manager_info* gDeviceManager; - +#ifdef TRACE_VIRTIO_INPUT static void WriteInputPacket(const VirtioInputPacket &pkt) { @@ -159,7 +159,7 @@ WriteInputPacket(const VirtioInputPacket &pkt) TRACE(", %" B_PRId32, pkt.value); } } - +#endif static void InitPackets(VirtioInputDevice* dev, uint32 count) @@ -208,26 +208,6 @@ PacketPhysEntry(VirtioInputDevice* dev, Packet* pkt) } -static Packet* -AllocPacket(VirtioInputDevice* dev) -{ - int32 idx = dev->freePackets; - if (idx < 0) - return NULL; - dev->freePackets = dev->packets[idx].next; - - return &dev->packets[idx]; -} - - -static void -FreePacket(VirtioInputDevice* dev, Packet* pkt) -{ - pkt->next = dev->freePackets; - dev->freePackets = pkt - dev->packets; -} - - static void ScheduleReadyPacket(VirtioInputDevice* dev, Packet* pkt) { diff --git a/src/add-ons/kernel/drivers/input/wacom/wacom.c b/src/add-ons/kernel/drivers/input/wacom/wacom.c index 773edcebc0..34ef083950 100644 --- a/src/add-ons/kernel/drivers/input/wacom/wacom.c +++ b/src/add-ons/kernel/drivers/input/wacom/wacom.c @@ -109,12 +109,12 @@ static wacom_device* add_device(usb_device dev) { wacom_device *device = NULL; - int num, ifc, alt; + int num; + size_t ifc, alt; const usb_interface_info *ii; status_t st; const usb_device_descriptor* udd; const usb_configuration_info *conf; - bool setConfiguration = false; // we need these four for a Wacom tablet size_t controlTransferLength; @@ -137,7 +137,6 @@ add_device(usb_device dev) // see if the device has been configured already if (!conf) { conf = usb->get_nth_configuration(dev, 0); - setConfiguration = true; } if (!conf) @@ -184,96 +183,93 @@ got_one: device->notify_lock = -1; device->data = NULL; -// if (setConfiguration) { - // the configuration has to be set yet (was not the current one) - DPRINTF_INFO((ID "add_device() - setting configuration...\n")); - if ((st = usb->set_configuration(dev, conf)) != B_OK) { + DPRINTF_INFO((ID "add_device() - setting configuration...\n")); + if ((st = usb->set_configuration(dev, conf)) != B_OK) { + dprintf(ID "add_device() -> " + "set_configuration() returns %" B_PRId32 "\n", st); + goto fail; + } else + DPRINTF_ERR((ID " ... success!\n")); + + if (conf->interface[ifc].active != ii) { + // the interface we found is not the active one and has to be set + DPRINTF_INFO((ID "add_device() - setting interface: %p...\n", ii)); + if ((st = usb->set_alt_interface(dev, ii)) != B_OK) { dprintf(ID "add_device() -> " - "set_configuration() returns %" B_PRId32 "\n", st); + "set_alt_interface() returns %" B_PRId32 "\n", st); goto fail; } else DPRINTF_ERR((ID " ... success!\n")); + } + // see if the device is a Wacom tablet and needs some special treatment + // let's hope Wacom doesn't produce normal mice any time soon, or this + // check will have to be more specific about product_id...hehe + if (udd->vendor_id == 0x056a) { + // do the control transfers to set up absolute mode (default is HID + // mode) - if (conf->interface[ifc].active != ii) { - // the interface we found is not the active one and has to be set - DPRINTF_INFO((ID "add_device() - setting interface: %p...\n", ii)); - if ((st = usb->set_alt_interface(dev, ii)) != B_OK) { - dprintf(ID "add_device() -> " - "set_alt_interface() returns %" B_PRId32 "\n", st); - goto fail; - } else - DPRINTF_ERR((ID " ... success!\n")); + // see 'Device Class Definition for HID 1.11' (HID1_11.pdf), + // par. 7.2 (available at www.usb.org/developers/hidpage) + + // set protocol mode to 'report' (instead of 'boot') + controlTransferLength = 0; + // HID Class-Specific Request, Host to device (=0x21): + // SET_PROTOCOL (=0x0b) to Report Protocol (=1) + // of Interface #0 (=0) + st = usb->send_request(dev, 0x21, 0x0b, 1, 0, 0, 0, + &controlTransferLength); + + if (st < B_OK) { + dprintf(ID "add_device() - " + "control transfer 1 failed: %" B_PRId32 "\n", st); } - // see if the device is a Wacom tablet and needs some special treatment - // let's hope Wacom doesn't produce normal mice any time soon, or this - // check will have to be more specific about product_id...hehe - if (udd->vendor_id == 0x056a) { - // do the control transfers to set up absolute mode (default is HID - // mode) - // see 'Device Class Definition for HID 1.11' (HID1_11.pdf), - // par. 7.2 (available at www.usb.org/developers/hidpage) + // try up to five times to set the tablet to 'Wacom'-mode (enabling + // absolute mode, pressure data, etc.) + controlTransferLength = 2; - // set protocol mode to 'report' (instead of 'boot') - controlTransferLength = 0; + for (tryCount = 0; tryCount < 5; tryCount++) { // HID Class-Specific Request, Host to device (=0x21): - // SET_PROTOCOL (=0x0b) to Report Protocol (=1) - // of Interface #0 (=0) - st = usb->send_request(dev, 0x21, 0x0b, 1, 0, 0, 0, - &controlTransferLength); + // SET_REPORT (=0x09) type Feature (=3) with ID 2 (=2) of + // Interface #0 (=0) to repData (== { 0x02, 0x02 }) + st = usb->send_request(dev, 0x21, 0x09, (3 << 8) + 2, 0, 2, + repData, &controlTransferLength); if (st < B_OK) { dprintf(ID "add_device() - " - "control transfer 1 failed: %" B_PRId32 "\n", st); + "control transfer 2 failed: %" B_PRId32 "\n", st); } - // try up to five times to set the tablet to 'Wacom'-mode (enabling - // absolute mode, pressure data, etc.) - controlTransferLength = 2; + // check if registers are set correctly - for (tryCount = 0; tryCount < 5; tryCount++) { - // HID Class-Specific Request, Host to device (=0x21): - // SET_REPORT (=0x09) type Feature (=3) with ID 2 (=2) of - // Interface #0 (=0) to repData (== { 0x02, 0x02 }) - st = usb->send_request(dev, 0x21, 0x09, (3 << 8) + 2, 0, 2, - repData, &controlTransferLength); + // HID Class-Specific Request, Device to host (=0xA1): + // GET_REPORT (=0x01) type Feature (=3) with ID 2 (=2) of + // Interface #0 (=0) to retData + st = usb->send_request(dev, 0xA1, 0x01, (3 << 8) + 2, 0, 2, + retData, &controlTransferLength); - if (st < B_OK) { - dprintf(ID "add_device() - " - "control transfer 2 failed: %" B_PRId32 "\n", st); - } - - // check if registers are set correctly - - // HID Class-Specific Request, Device to host (=0xA1): - // GET_REPORT (=0x01) type Feature (=3) with ID 2 (=2) of - // Interface #0 (=0) to retData - st = usb->send_request(dev, 0xA1, 0x01, (3 << 8) + 2, 0, 2, - retData, &controlTransferLength); - - if (st < B_OK) { - dprintf(ID "add_device() - " - "control transfer 3 failed: %" B_PRId32 "\n", st); - } - - DPRINTF_INFO((ID "add_device() - retData: %u - %u\n", - retData[0], retData[1])); - - if (retData[0] == repData[0] && retData[1] == repData[1]) { - DPRINTF_INFO((ID "add_device() - successfully set " - "'Wacom'-mode\n")); - break; - } + if (st < B_OK) { + dprintf(ID "add_device() - " + "control transfer 3 failed: %" B_PRId32 "\n", st); } - DPRINTF_INFO((ID "add_device() - number of tries: %u\n", - tryCount + 1)); + DPRINTF_INFO((ID "add_device() - retData: %u - %u\n", + retData[0], retData[1])); - if (tryCount > 4) { - dprintf(ID "add_device() - set 'Wacom'-mode failed\n"); + if (retData[0] == repData[0] && retData[1] == repData[1]) { + DPRINTF_INFO((ID "add_device() - successfully set " + "'Wacom'-mode\n")); + break; } } -// } + + DPRINTF_INFO((ID "add_device() - number of tries: %u\n", + tryCount + 1)); + + if (tryCount > 4) { + dprintf(ID "add_device() - set 'Wacom'-mode failed\n"); + } + } // configure the rest of the wacom_device device->pipe = ii->endpoint[0].handle; @@ -479,16 +475,16 @@ device_free(void *cookie) return B_OK; } -// device_interupt_callback +// device_interrupt_callback static void -device_interupt_callback(void* cookie, status_t status, void* data, - uint32 actualLength) +device_interrupt_callback(void* cookie, status_t status, void* data, + size_t actualLength) { wacom_device* device = (wacom_device*)cookie; - uint32 length = min_c(actualLength, device->max_packet_size); + size_t length = min_c(actualLength, device->max_packet_size); - DPRINTF_INFO((ID "device_interupt_callback(%p) name = \"%s%d\" -> " - "status: %ld, length: %ld\n", cookie, kBasePublishPath, device->number, + DPRINTF_INFO((ID "device_interrupt_callback(%p) name = \"%s%d\" -> " + "status: %ld, length: %zu\n", cookie, kBasePublishPath, device->number, status, actualLength)); device->status = status; @@ -502,7 +498,7 @@ device_interupt_callback(void* cookie, status_t status, void* data, release_sem(device->notify_lock); } - DPRINTF_INFO((ID "device_interupt_callback() - done\n")); + DPRINTF_INFO((ID "device_interrupt_callback() - done\n")); } // read_header @@ -548,7 +544,7 @@ device_read(void* cookie, off_t pos, void* buf, size_t* count) if (*count > sizeof(wacom_device_header)) { // queue the interrupt transfer ret = usb->queue_interrupt(device->pipe, device->data, - device->max_packet_size, device_interupt_callback, device); + device->max_packet_size, device_interrupt_callback, device); if (ret >= B_OK) { // we will block here until the interrupt transfer has been done ret = acquire_sem_etc(device->notify_lock, 1,