On Thu, Nov 15, 2018 at 04:26:33PM -0800, Tadeusz Struk wrote: > 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. Point taken. > > 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. 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
