On Fri, 2024-07-19 at 16:21 +0300, Jarkko Sakkinen wrote: > On Wed, 2024-07-17 at 22:30 -0400, James Bottomley wrote: > > On Tue, 2024-07-16 at 17:07 +0300, Jarkko Sakkinen wrote: > > > On Tue Jul 16, 2024 at 2:53 PM EEST, Jarkko Sakkinen wrote: > > > > > - u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE]; > > > > > + u8 name[AUTH_MAX_NAMES][2 + HASH_MAX_DIGESTSIZE]; > > > > > > Ouch, we definitely do not want 2-dimensional arrays. I missed this > > > in the hmac review. > > > > > > Why this is based on count (AUTH_MAX_NAMES) rather than space? Is > > > that value from the specs? > > > > Yes, it's based on the maximum number of session handles a command can > > have. It's architecturally defined in Trusted Platform Module Library > > Part 1: Architecture chapter 18 (TPM Command/Response Structure) where > > it says in 18.1 "an Authorization Area containing one to three session > > structures" > > > > Although if I look at our code we really only use a maximum of two for > > all the commands the kernel does. > > > > > You could just as well replace name and name_h with a single tpm_buf > > > instance in "sized" mode and return -E2BIG from the functions that > > > use it. Right, those don't return anything but void, which should be > > > also fixed. > > > > I'll look into that: it would get us out of the buf->handles spat. > > > Also one thing I recalled after reviewing this: when updating > any of the exported functions: > > 1. Try to scope patch per function. Obviously when sanely > possible but at least goal should be this granularity. > 2. It would make sense to take the documentation from header > and kdoc and merge them into a single entity. > > This way I think it would be more digestable and easier both > test and review. My philosophy here is like for any other new feature: 1. The first version can be messier, and all the issues I've complained about (mostly because I came from holiday to fix them) are kind of part of the job. 2. The second version or supplemental feature should make the code base cleaner than it was before it existed. So I kind of see the second iteration also a chance fix all the glitches and rough corners. I don't think anything was done "wrong" durign the first iteration, now it is just easier to reflect some of the choices. BR, Jarkko
