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
--
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
