Re: [Qemu-devel] [PATCH 1/3] usb-mtp: use a list for keeping track of children

[Bandan Das <bsd@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> writes:

>>  static void usb_mtp_object_free(MTPState *s, MTPObject *o)
>>  {
>> -    int i;
>> +    MTPObject *iter;
>> +
>> +    if (o) {
>> +        trace_usb_mtp_object_free(s->dev.addr, o->handle, o->path);
>
> That also makes usb_mtp_object_free callable with o == NULL.  Makes
> sense, but also makes this patch hard to review.  Please consider either
> splitting this out into a separate patch or code this as "if (!o)
> { return; }" so the intention of the remaining code doesn't change.

Sure, I will split up the call to usb_mtp_object_free() during reset
to a separate patch and change the check here to if (!o).

Bandan

>> +static MTPObject *usb_mtp_add_child(MTPState *s, MTPObject *o,
>> +                              char *name)
>> +{
>> +    MTPObject *child =
>> +        usb_mtp_object_alloc(s, s->next_handle++, o, name);
>> +
>> +    if (child) {
>> +        trace_usb_mtp_add_child(s->dev.addr, child->handle, child->path);
>> +        QLIST_INSERT_HEAD(&o->children, child, list);
>> +        o->nchildren++;
>> +
>> +        if (child->format == FMT_ASSOCIATION) {
>> +            QLIST_INIT(&child->children);
>> +        }
>> +
>> +    }
>> +
>> +    return child;
>> +}
>
> Separate patch please.
>
> cheers,
>   Gerd