Re: [Qemu-devel] [PATCH 1/8] usb: Add packet combining functions

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/01/2012 11:08 AM, Gerd Hoffmann wrote:
> On 10/31/12 13:47, Hans de Goede wrote:
>> +    /*
>> +     * If we had leftover packets the hcd driver will have cancelled them
>> +     * and usb_combined_packet_cancel has already freed combined!
>> +     */
>> +    if (state != leftover) {
>> +        g_free(combined);
>> +    }
>
> This calls for reference-counting USBCombinedPacket IMHO.

Why? We call packet_complete with a status if USB_RET_REMOVE_FROM_QUEUE
if we've left-over packets, the hcd code will cancel these, and
usb_combined_packet_cancel will free the combined packet when the
last packet of it gets cancelled, which *will* happen as we're
always processing *all* packets in combined here. There is no
scenario here where one or the other party wants to keep the
combined packet around any longer...

The only reason this is a bit non straightforward is that
normally packets get freed either on completion or cancellation,
but here we've a partial completion and a partial cancellation.

Also can you please just do one review and then point out all the
issues you see? Esp. since the feature freeze for 1.3 is *today*

Regards,

Hans