Re: [PATCH v3] tpm: add support for partial reads

[Tadeusz Struk <tadeusz.struk@intel.com>]

On 11/15/18 3:31 PM, Jarkko Sakkinen wrote:
> You could drop these memset() calls and also one from
> tpm_timeout_work(). The call could be done once in the beginning of
> tpm_common_write() instead of having three different call sites.
> 

Don't we want to clean the buffer as the response is read?
When we will only memset it in write and if one would send
just one command there will only be one response.
The data will sit in the buffer until the next command.
There will be a quite bit time window when the data can leak.

> 
> Naming becomes a mess and the comment for data_pending variable is
> incorrect as it is also used for synchronous operation.
> 
> Maybe add a prepending commit to rename it as
> 
> 	/* Holds the resul of the tpm_transmit() last call. */
> 	ssize_t transmit_result;

Agree, will do that.

> 
> That is at least clear and obvious on what it contains.
> 
> The comment for partial_data is incorrect as the variable does not
> contain any data.

Yes, I will change it.

> 
> We could use declare:
> 
> 	/* Holds the count how much of the response is still unread. */
> 	size_t response_pending;
> 
> Observe another remark from your commit: there is no reaso to ssize_t as
> the type as the value should never be a negative number.

Yes, in fact now the priv->data_pending can also be only positive.
I'll change it and send a new version soon.
Thanks,
-- 
Tadeusz