Re: [PATCH_v4 1/5] android/hid: Fix set seport ipc cmd preparation

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

Hi Johan,

On 11/08/2013 02:15 PM, Johan Hedberg wrote:
> Hi Ravi,
>
> On Fri, Nov 08, 2013, Ravi kumar Veeramally wrote:
>> -	memcpy(cmd.data, report, cmd.len);
>> +	memset(cmd, 0, sizeof(buf));
> How about memset(buf, 0, sizeof(buf)) to make this perfectly clear.
   Ok.
>   Btw,
> why is the memset needed? I don't think the original code had it. Seems
> like this is an independent bug fix?
   Not in original, but in between versions (v1~v4) :(.
>
>> +	cmd->len = strlen(report);
>> +	memcpy(cmd->data, report, cmd->len);
>>   
>>   	switch (report_type) {
>>   	case BTHH_INPUT_REPORT:
>> -		cmd.type = HAL_HIDHOST_INPUT_REPORT;
>> +		cmd->type = HAL_HIDHOST_INPUT_REPORT;
>>   		break;
>>   	case BTHH_OUTPUT_REPORT:
>> -		cmd.type = HAL_HIDHOST_OUTPUT_REPORT;
>> +		cmd->type = HAL_HIDHOST_OUTPUT_REPORT;
>>   		break;
>>   	case BTHH_FEATURE_REPORT:
>> -		cmd.type = HAL_HIDHOST_FEATURE_REPORT;
>> +		cmd->type = HAL_HIDHOST_FEATURE_REPORT;
>>   		break;
>>   	default:
>>   		return BT_STATUS_PARM_INVALID;
>>   	}
>>   
>>   	return hal_ipc_cmd(HAL_SERVICE_ID_HIDHOST, HAL_OP_HIDHOST_SET_REPORT,
>> -				sizeof(cmd), &cmd, 0, NULL, NULL);
>> +					sizeof(buf), buf, 0, NULL, NULL);
> This last call looks broken to me. Shouldn't you instead of sizeof(buf)
> be sending sizeof(*cmd) + cmd->len?
   yes, should be "sizeof(*cmd) + cmd->len".
>
> Johan
>
Thanks,
Ravi.