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

[Ravi Kumar Veeramally <ravikumar.veeramally@linux.intel.com>]

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.