On Thu, Nov 15, 2018 at 04:26:33PM -0800, Tadeusz Struk wrote:

On 11/15/18 3:31 PM, Jarkko Sakkinen wrote:

quoted

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.

Point taken.

quoted

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.

quoted

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.

quoted

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.

Right you're correct. Then it should be simply called as response_size
(and be size_t) as it is then the most precise name for what it
contains.

Thanks,
-- 
Tadeusz

/Jarkko