Re: [PATCH v2] HID: leave dev_rdesc unmodified and use it for comparisons
From: Henrik Rydberg <hidden>
Date: 2012-09-19 18:32:34
Also in:
linux-input
Hi Kevin, Thanks for looking to this.
Hmm. I hadn't noticed that the other drivers are returning a static structure. In that case, it seems that report_fixup itself is broken from a memory perspective, in that it returns pointers to inconsistent storage types depending on the driver.
The driver can either modify the existing buffer, of return a pointer to a buffer managed by the driver. The former requires a kmemdup before, the latter a kmemdup after.
1. Ugly workaround: make a temporary copy of the dev_rdesc, give it to report_fixup, make a copy of the return, store that copy in rdesc, free the temporary copy. Though ugly, this would at least involve the smallest diff.
Yes, it is correct and ugly, in no particular order.
2. Standardize the behavior of the drivers' report_fixup implementations. Given that some of them need to change the size of the descriptor, modifying the passed structure is not an option. Probably all of them should return a newly allocated structure, either a modified copy of the input or a copy of their static, that can then be stored directly in rdesc. Especially since report_fixup is only ever called right before a copy is going to be taken anyway.
Relying on the returned pointer to be properly alloc'd is not a good idea, in particular since it changes semantics rather drastically. Since the current function performs two different things, perhaps there should be two different functions instead.
(Adding constness to a parameter isn't considered a severe ABI break, is it?)
Inside the kernel there is no ABI. Go wild. Thanks, Henrik