On 09/08/2015 08:58 PM, Greg KH wrote:
On Tue, Sep 08, 2015 at 06:10:16PM -0700, Guenter Roeck wrote:
Hi Emilio,
On 09/08/2015 05:51 PM, Emilio López wrote:
Hi Greg & Guenter,
[ ... ]
Unless I am missing something, this is not explained anywhere, but it is
not entirely trivial to understand. I think it should be documented.
I agree. I couldn't find any mention of what this int was supposed to be by looking at Documentation/ (is_visible is not even mentioned :/) or include/linux/sysfs.h. Once we settle on something I'll document it before sending a v2.
In the include file ? No strong preference, though.
By the way, I wrote a quick coccinelle script to match is_visible() users which reference the index (included below), and it found references to drivers which do not seem to use any binary attributes, so I believe changing the index meaning shouldn't be an issue.
Good.
I agree, make i the number of the bin attribute and that should solve
this issue.
No, that would conflict with the "normal" use of is_visible for non-binary
attributes, and make the index all but useless, since the is_visible function
would have to search through all the attributes anyway to figure out which one
is being checked.
Yeah, using the same indexes would be somewhat pointless, although not many seem to be using it anyway (only 14 files matched). Others seem to be comparing the attr* instead. An alternative would be to use negative indexes for binary attributes and positive indexes for normal attributes.
... and I probably wrote or reviewed a significant percentage of those ;-).
Using negative numbers for binary attributes is an interesting idea.
Kind of unusual, though. Greg, any thoughts on that ?
Ick, no, that's a mess, maybe we just could drop the index alltogether?
No, please don't. Having to manually compare dozens of index pointers would be
even more of a mess.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
