Re: [Qemu-devel] [PATCH 2/5] tpm: pull tpm_util_send() out of tpm_util_test()

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

Hi Stefan,

On Mon, Nov 06, 2017 at 07:58:53PM -0500, Stefan Berger wrote:
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  hw/tpm/tpm_util.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c
> index daf1faa..396e793 100644
> --- a/hw/tpm/tpm_util.c
> +++ b/hw/tpm/tpm_util.c
> @@ -53,10 +53,11 @@ bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len)
>   * A basic test of a TPM device. We expect a well formatted response header
>   * (error response is fine) within one second.
>   */
> -static int tpm_util_test(int fd,
> -                         unsigned char *request,
> -                         size_t requestlen,
> -                         uint16_t *return_tag)
> +static int tpm_util_tx(int fd,

The function is not just tx, but write & read. Call it
tpm_util_request() instead?

> +                       unsigned char *request,
> +                       size_t requestlen,
> +                       unsigned char *response,
> +                       size_t responselen)
>  {
>      struct tpm_resp_hdr *resp;
>      fd_set readfds;
> @@ -65,7 +66,6 @@ static int tpm_util_test(int fd,
>          .tv_sec = 1,
>          .tv_usec = 0,
>      };
> -    unsigned char buf[1024];
>  
>      n = write(fd, request, requestlen);
>      if (n < 0) {
> @@ -84,17 +84,36 @@ static int tpm_util_test(int fd,
>          return -errno;
>      }
>  
> -    n = read(fd, &buf, sizeof(buf));
> +    n = read(fd, response, responselen);
>      if (n < sizeof(struct tpm_resp_hdr)) {
>          return -EFAULT;
>      }
>  
> -    resp = (struct tpm_resp_hdr *)buf;
> +    resp = (struct tpm_resp_hdr *)response;
>      /* check the header */
>      if (be32_to_cpu(resp->len) != n) {
>          return -EMSGSIZE;
>      }
>  
> +    return 0;
> +}
> +
> +static int tpm_util_test(int fd,
> +                         unsigned char *request,
> +                         size_t requestlen,
> +                         uint16_t *return_tag)
> +{
> +    struct tpm_resp_hdr *resp;
> +    unsigned char buf[1024];
> +    ssize_t ret;
> +
> +    ret = tpm_util_tx(fd, request, requestlen,
> +                      buf, sizeof(buf));
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    resp = (struct tpm_resp_hdr *)buf;
>      *return_tag = be16_to_cpu(resp->tag);
>  
>      return 0;
> -- 
> 2.5.5
> 
> 

Looks fine otherwise,

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>