Re: [PATCH v4 2/7] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Tue, 23 Oct 2018, Ard Biesheuvel wrote:
> On 23 October 2018 at 04:01, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
>> On Mon, 2018-10-22 at 19:19 -0300, Ard Biesheuvel wrote:
>> [...]
>>>> +static void hmac_init(struct shash_desc *desc, u8 *key, int
>>>> keylen)
>>>> +{
>>>> +       u8 pad[SHA256_BLOCK_SIZE];
>>>> +       int i;
>>>> +
>>>> +       desc->tfm = sha256_hash;
>>>> +       desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
>>>
>>> I don't think this actually does anything in the shash API
>>> implementation, so you can drop this.
>>
>> OK, I find crypto somewhat hard to follow.  There were bits I had to
>> understand, like when I wrote the CFB implementation or when I fixed
>> the ECDH scatterlist handling, but I've got to confess, in time
>> honoured tradition I simply copied this from EVM crypto without
>> actually digging into the code to understand why.
>>
>
> Yeah, it is notoriously hard to use, and we should try to improve that.

James,

I would hope (already said in my review) to use longer than one
character variable names for most of the stuff. I did not quite
understand why you decided to use 'counter' for obvious counter
variable and one character names for non-obvious stuff :-)

I'm not sure where the 'encoded' exactly comes in the variable
name 'encoded_key' especially in the context of these cryptic
names.

/Jarkko