Re: [Qemu-devel] [PATCH] Add TPM support

[Fabrice Bellard]

Thomas Bleher wrote:
> * Fabrice Bellard <address@hidden> [2007-11-05 16:40]:
>> Thomas Bleher wrote:
>>> Thiemo Seufer told me that GPLv2 is fine for qemu, therefore I'd like to
>>> ask that this patch be included in qemu as I posted it (the second
>>> version with the clarified GPLv2 license).
>> I prefer that a BSD style license is used, especially if the code just 
>> contains wrappers.
> 
> Fine with me, too.

OK.

> +        result = write(s->tpm_fd, s->send_data, s->send_data_index);
> +        if (result < s->send_data_index) {
> +            fprintf(stderr, "WARNING: Failed to write data to tpm!\n");
> +            return ATML_STATUS_BUSY;
> +        }

You should handle EINTR and EAGAIN.

> +        s->send_data_index = 0;
> +        s->recv_data_pos = 0;
> +        s->recv_data_length = 0;
> +        s->data_to_send = 0;
> +        s->data_to_recv = 1;
> +    }
> +    if (s->data_to_recv) {
> +        if (poll(&(s->tpm_poll), 1, 0) > 0) {

poll is not needed as read will block.

> +            result = read(s->tpm_fd, s->recv_data, 2048);
> +            if (result < 6) { // a minimal packet is 6 bytes long
> +                fprintf(stderr, "WARNING: Not enough data from tpm!\n");
> +                return ATML_STATUS_BUSY;
> +            }

same comment as write().

> +    res = connect(sock, (struct sockaddr*)&addr, sizeof(struct sockaddr_un));
> +    if (res < 0)
> +        return -1;

same comment for EINTR.

+/* should be called first, initializes all structures and connects to
the external emulator */
+void tpm_configure(const char* tpm_socket)
+{

Suppress this function (cf tpm_register).

> +/* split of from tpm_configure() so the configuration can be called earlier 
> */
> +void tpm_register()

Ansi C requires void in parameters. Allocate the state and return it.
Suppress the global variable tpm_state. Pass the path to this function
and suppress the configure function. Add a global variable for the path
and register the device if it is not NULL.

Regards,

Fabrice.