Re: [Qemu-devel] [PATCH 2/2 v2] usb-mtp: refactor the flow of usb_mtp_write_data

[Bandan Das]

Peter Maydell <address@hidden> writes:

> On Tue, 26 Mar 2019 at 17:58, Bandan Das <address@hidden> wrote:
>>
>
...
> Doesn't this mean you're no longer sending the RES_OK result
> for this case ?
>
Ugh, I messed up this version. I will send a v3 and
fix the indentation too. Thanks for noticing!

Bandan

>>              }
>>              if (d->length != MTP_WRITE_BUF_SZ && !d->pending) {
>>                  d->write_status = WRITE_END;
>> @@ -1647,13 +1653,12 @@ static int usb_mtp_write_data(MTPState *s)
>>          rc = write_retry(d->fd, d->data, d->data_offset,
>>                           d->offset - d->data_offset);
>>          if (rc != d->data_offset) {
>> -            usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
>> -                                 0, 0, 0, 0);
>> +            ret = 1;
>>              goto done;
>>          }
>>          if (d->write_status != WRITE_END) {
>>              g_free(path);
>> -            return ret;
>> +            return;
>>          } else {
>>              /*
>>               * Return an incomplete transfer if file size doesn't match
>> @@ -1665,16 +1670,17 @@ static int usb_mtp_write_data(MTPState *s)
>>                  usb_mtp_update_object(parent, s->dataset.filename)) {
>>                  usb_mtp_queue_result(s, RES_INCOMPLETE_TRANSFER, d->trans,
>>                                       0, 0, 0, 0);
>> -                goto done;
>> +                goto close;
>>              }
>>          }
>
> Similarly the code path which succeeds and falls out of the end
> of this case will no longer be sending RES_OK, I think.
>
>>      }
>>
>> -success:
>> -    usb_mtp_queue_result(s, RES_OK, d->trans,
>> -                         0, 0, 0, 0);
>> -
>>  done:
>> +    if (ret) {
>> +        usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
>> +                             0, 0, 0, 0);
>> +    }
>> +close:
>>      /*
>>       * The write dataset is kept around and freed only
>>       * on success or if another write request comes in
>> @@ -1683,12 +1689,10 @@ done:
>>          close(d->fd);
>>          d->fd = -1;
>>      }
>> -free:
>>      g_free(s->dataset.filename);
>>      s->dataset.size = 0;
>>      g_free(path);
>>      s->write_pending = false;
>> -    return ret;
>>  }
>
> thanks
> -- PMM