Re: [PATCH_v2 02/11] android/hid: Add a ascii2hex utility

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/08/2013 10:48 AM, Johan Hedberg wrote:
Hi Ravi,

On Fri, Nov 08, 2013, Ravi Kumar Veeramally wrote:
+void ascii2hex(const uint8_t *ascii, int ascii_len, uint8_t *hex)
+{
+	int i;
+
+	if (!ascii || !hex)
+		return;
+
+	for (i = 0; i < ascii_len / 2; i++)
+		sscanf((char *) &ascii[i * 2], "%02x",
+						(unsigned int *) &hex[i]);
+
+}
I'd just keep this static inside the HID HAL since that seems to be
the only user for it. Actually I'm not even convinced that it's worth
to have this as a separate function since you only have two users and
essentially the function is just two lines (the if-statement is
redundant).
Ok, I will keep it as static. Do you want me to keep in HID HAL or HID
daemon?
The daemon is where you were doing the conversion now as well, so no
need to change that (sorry about being unclear when I said "HID HAL").

Also, the function name isn't quite right. You're converting from hex
to binary
  We are converting to hex right? correct me If I am wrong.
No, we're converting from hex to binary.

. Not from ascii to hex (hex notation is already using
the ascii character set). Also, do consider the point from Jerzy about
the format specifier.

  Ok, I will fix Jerzy's suggestion also.

Apart from this patch, do you have any comments on rest of the
patches?
Only the ones I already sent.

Johan

 Ok, Patch_v2 6 & 7 are independent of ascii stuff, can be it possible to
 apply if there are no comments? (just a request!!).

 Thanks a lot,
 Ravi.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux