[PATCH v2 1/2] HID: logitech-hidpp: fix negated returns

* [PATCH v2 1/2] HID: logitech-hidpp: fix negated returns
@ 2014-11-03 21:09 Benjamin Tissoires
  2014-11-03 21:09 ` [PATCH v2 2/2] HID: logitech-hidpp: 2 fixes in hidpp_root_get_protocol_version() Benjamin Tissoires
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Benjamin Tissoires @ 2014-11-03 21:09 UTC (permalink / raw)
  To: Jiri Kosina, Dan Carpenter, Nestor Lopez Casado; +Cc: linux-input, linux-kernel

Reported by Dan Carpenter:

drivers/hid/hid-logitech-hidpp.c:359 hidpp_root_get_protocol_version() warn: should this return really be negated?
drivers/hid/hid-logitech-hidpp.c:398 hidpp_devicenametype_get_count() warn: should this return really be negated?
drivers/hid/hid-logitech-hidpp.c:417 hidpp_devicenametype_get_device_name() warn: should this return really be negated?
drivers/hid/hid-logitech-hidpp.c:524 hidpp_touchpad_get_raw_info() warn: should this return really be negated?

The problem lies in hidpp_send_message_sync() which can return 2 types of
errors depending of their sign. Adding a comment there to clarify what is
happening.

To solve that, print an error in case of a protocol problem, and raise
-EPROTO instead.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Changes in v2:
- also return if the ret code is negative

 drivers/hid/hid-logitech-hidpp.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 8d2d54b..61f9e75 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -151,6 +151,14 @@ static int __hidpp_send_report(struct hid_device *hdev,
 	return ret == fields_count ? 0 : -1;
 }
 
+/**
+ * hidpp_send_message_sync() returns 0 in case of success, and something else
+ * in case of a failure.
+ * - If ' something else' is positive, that means that an error has been raised
+ *   by the protocol itself.
+ * - If ' something else' is negative, that means that we had a classic error
+ *   (-ENOMEM, -EPIPE, etc...)
+ */
 static int hidpp_send_message_sync(struct hidpp_device *hidpp,
 	struct hidpp_report *message,
 	struct hidpp_report *response)
@@ -359,8 +367,13 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 		return 0;
 	}
 
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
 	if (ret)
-		return -ret;
+		return ret;
 
 	hidpp->protocol_major = response.fap.params[0];
 	hidpp->protocol_minor = response.fap.params[1];
@@ -398,8 +411,13 @@ static int hidpp_devicenametype_get_count(struct hidpp_device *hidpp,
 	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
 		CMD_GET_DEVICE_NAME_TYPE_GET_COUNT, NULL, 0, &response);
 
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
 	if (ret)
-		return -ret;
+		return ret;
 
 	*nameLength = response.fap.params[0];
 
@@ -417,8 +435,13 @@ static int hidpp_devicenametype_get_device_name(struct hidpp_device *hidpp,
 		CMD_GET_DEVICE_NAME_TYPE_GET_DEVICE_NAME, &char_index, 1,
 		&response);
 
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
 	if (ret)
-		return -ret;
+		return ret;
 
 	if (response.report_id == REPORT_ID_HIDPP_LONG)
 		count = HIDPP_REPORT_LONG_LENGTH - 4;
@@ -524,8 +547,13 @@ static int hidpp_touchpad_get_raw_info(struct hidpp_device *hidpp,
 	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
 		CMD_TOUCHPAD_GET_RAW_INFO, NULL, 0, &response);
 
+	if (ret > 0) {
+		hid_err(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return -EPROTO;
+	}
 	if (ret)
-		return -ret;
+		return ret;
 
 	raw_info->x_size = get_unaligned_be16(&params[0]);
 	raw_info->y_size = get_unaligned_be16(&params[2]);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread