On Tue, Sep 08, 2020 at 11:14:11AM -0700, James Bottomley wrote: > On Tue, 2020-09-08 at 21:05 +0300, Jarkko Sakkinen wrote: > > On Tue, Sep 08, 2020 at 07:45:52AM +0200, Greg KH wrote: > > > On Mon, Sep 07, 2020 at 02:52:08PM -0700, James Bottomley wrote: > [...] > > > > I've got to say I think binary attributes are actively evil. I > > > > can see > > > > they're a necessity when there's no good way to represent the > > > > data they > > > > contain, like the bios measurement log or firmware code or a raw > > > > interface like we do for the SMP frame code in libsas. But when > > > > there's a well understood and easy to produce user friendly non- > > > > binary > > > > representation, I think dumping binary is inimical to being a > > > > good API. > > > > > > Agreed. > > > > > > thanks, > > > > > > greg k-h > > > > Looking at the patch, something like <device>/pcrs/<hash>/<index> > > would be a bit cleaner representation than the current <device>/pcrs- > > <hash>/<index>. > > That's actually a technical limitation of using the current attribute > groups API: It's designed to support single level directories in sysfs > (or no directory at all). That's not to say we can't do multi-level > ones, but if we do we have to roll our own machinery for managing the > files rather than relying on the groups API. > > Given that the current groups API does all the nasty lifetime > management that I'd otherwise have to do in the patch, I have a strong > incentive for keeping it, which is why the single <device>/pcrs- > <hash>/<index> format. OK, I do get that. I've tried to do something similar in past and it turnd out to be a tremendous job. I guess I'll have to re-review the whole patch again. I'll prioritize that next week. > James /Jarkko
