Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 19 Nov 2018 15:58:00 +0200
From:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To:     Tadeusz Struk <tadeusz.struk@intel.com>
Cc:     jgg@ziepe.ca, linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 2/2] tpm: add support for partial reads
Message-ID: <20181119135800.GE8755@linux.intel.com>
References: <154239067364.2880.13752725056462237016.stgit@tstruk-mobl1.jf.intel.com>
 <154239071144.2924.17574189056250189697.stgit@tstruk-mobl1.jf.intel.com>
 <20181118074832.GG5897@linux.intel.com>
 <90072f09-6388-b1f7-387b-42f056f8893d@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <90072f09-6388-b1f7-387b-42f056f8893d@intel.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, Nov 18, 2018 at 07:05:19PM -0800, Tadeusz Struk wrote:
> > When ret_size < 0? Shouldn't this be just "if (!ret_size)"?
> 
> What we want to check here is if ret_size is positive, which is a valid
> value, or if it is negative or zero, which is an invalid value, so in
> this case (!ret_size) will not work.

Please explain a scenario where "!ret_size" would no work given that
both size and partial_data have always positive value?

> >> 	/* Holds the resul of the last successful call to tpm_transmit() */
> >>  	size_t transmit_result;
> >> +	/* Holds the count how much of the response is still unread */
> >> +	size_t partial_data;
> > I'm otherwise happy how this look like but why call it partial_data.
> > You cannot really tell from the name anything about its contents as
> > data is very abstract term.
>  
> so I will rename these two to response_length and response_length_rem,
> how does this sound?

Yes, assuming that there would be a hard requirement to even have two
variables in the first place.

> > BTW, why you need the new variable anyway and not just decrease the
> > variable where the length is original stored?
> 
> We need to have two variables, otherwise how do we tell if some part of
> response was consumed to allow sending a new command?

I don't understand. In order to maintain backwards compatibility you can
send a new command at any time.

> The transmit_result is used for that. If it is zero then one can transmit
> a new command even if the whole response is not consumed. The new variable
> tracks how much of the response is still to be read. 

AFAIK you only need to track the latter, not both.

/Jarkko