[Spice-devel] [PATCH] Fix leaks

[Hans de Goede]

Hi,

On 07/16/2014 01:24 AM, Fabiano Fidêncio wrote:
> ---
>  usbredirhost/usbredirhost.c             | 1 +
>  usbredirtestclient/usbredirtestclient.c | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
> index bbaafa4..6ab6e1b 100644
> --- a/usbredirhost/usbredirhost.c
> +++ b/usbredirhost/usbredirhost.c
> @@ -1191,6 +1191,7 @@ static void usbredirhost_alloc_stream_unlocked(struct usbredirhost *host,
>                  host->endpoint[EP2I(ep)].transfer[i], INTERRUPT_TIMEOUT);
>              break;
>          }
> +        free(buffer);
>      }
>      host->endpoint[EP2I(ep)].out_idx = 0;
>      host->endpoint[EP2I(ep)].drop_packets = 0;

Erm, no this wrong, very very wrong, you're freeing the buffer were using to store the actual
usb data in here, while the usb transfers are still being used (are being created to get
used) not good.

This will get freed when the actual transfer gets free-ed through calling
usbredirhost_free_transfer(), specifically by this line in that function:

    free(transfer->transfer->buffer);

> diff --git a/usbredirtestclient/usbredirtestclient.c b/usbredirtestclient/usbredirtestclient.c
> index 42b16dc..32fcba2 100644
> --- a/usbredirtestclient/usbredirtestclient.c
> +++ b/usbredirtestclient/usbredirtestclient.c
> @@ -404,6 +404,9 @@ static int usbredirtestclient_cmdline_ctrl(void)
>      }
>      usbredirparser_send_control_packet(parser, id, &control_packet,
>                                         data, data_len);
> +    if (data) {
> +        free(data);
> +    }
>      printf("Send control packet with id: %u\n", id);
>      id++;
>      return 1;

This leak fix is correct, no need for the if though, free(NULL) is a nop. I've pushed this
to the usbredir git repo.

Regards,

Hans