On Tue, Jul 1, 2014 at 11:14 AM, Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
> the unaligned helpers will do right thing. So no need for casting ourselves to death here. In addition you might want to think about the endianess here. And the unaligned helpers with the automatic endian conversion are nice to keep the code readable. Let the compiler figure out the details for you.
The following code should be more correct now but I still wonder if
I'm missing something regarding the use of "unaligned helpers". Is
this what you had in mind?
unsigned char *chip_id_base, *patch_length_base, *patch_offset_base;
chip_id_base = epatch_fw->data + sizeof(struct rtk_epatch_header);
patch_length_base = chip_id_base + (sizeof(u16) * epatch_info->num_patches);
patch_offset_base = patch_length_base + (sizeof(u16) *
epatch_info->num_patches);
for (i = 0; i < epatch_info->num_patches; i++) {
u16 chip_id = get_unaligned_le16(chip_id_base + (i * sizeof(u16)));
if (chip_id == eversion + 1) {
patch_length = get_unaligned_le16(patch_length_base + (i *
sizeof(u16));
patch_offset = get_unaligned_le32(patch_offset_base + (i *
sizeof(u32));
break;
}
}
>> + fw_len = rtk_load_firmware(hdev, &fw_blob, variant, eversion);
>> + if (fw_len < 0)
>> + return fw_len;
>
> No. This function returns either 0 or an error. Don't confuse the Bluetooth core with some random number.
I checked the return paths from rtk_load_firmware and they will always
return >0 or an error.
So the return shown above won't return random numbers, instead it will
return common errors like -ENOMEM.
Or would you prefer that I ignore the error code from
rtk_load_firmware() and just return a fixed error (EIO?) in such case?
Another alternative is to return 0 like the broadcom and intel code
does, on the offchance that the device is already running a usable
firmware version. What do you think?
Thanks
Daniel
--
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
