Thread (6 messages) 6 messages, 5 authors, 2012-09-19

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help