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

[Peter Maydell]

On Tue, 19 Mar 2019 at 21:46, Bandan Das <address@hidden> wrote:
>
> There's no functional change but the flow is (hopefully)
> more consistent for both file and folder object types.
>
> Signed-off-by: Bandan Das <address@hidden>
> ---
>  hw/usb/dev-mtp.c | 58 +++++++++++++++++++++++++-----------------------
>  1 file changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 4dc1317e2e..ced3a22384 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1599,7 +1599,7 @@ static int usb_mtp_update_object(MTPObject *parent, 
> char *name)
>      return ret;
>  }
>
> -static int usb_mtp_write_data(MTPState *s)
> +static void usb_mtp_write_data(MTPState *s, uint32_t handle)
>  {
>      MTPData *d = s->data_out;
>      MTPObject *parent =
> @@ -1607,7 +1607,7 @@ static int usb_mtp_write_data(MTPState *s)
>      char *path = NULL;
>      uint64_t rc;
>      mode_t mask = 0644;
> -    int ret = 0;
> +    int ret = 0, arg4 = 0, arg5 = 0, arg6 = 0, arg7 = 0;
>
>      assert(d != NULL);
>
> @@ -1616,26 +1616,32 @@ static int usb_mtp_write_data(MTPState *s)
>          if (!parent || !s->write_pending) {
>              usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO, d->trans,
>                  0, 0, 0, 0);
> -        return 1;
> +        return;
>          }
>
>          if (s->dataset.filename) {
>              path = g_strdup_printf("%s/%s", parent->path, 
> s->dataset.filename);
>              if (s->dataset.format == FMT_ASSOCIATION) {
>                  ret = mkdir(path, mask);
> -                goto free;
> +                if (!ret) {
> +                    arg4 = 3;
> +                    arg5 = QEMU_STORAGE_ID;
> +                    arg6 = s->dataset.parent_handle;
> +                    arg7 = handle;
> +                }
> +                goto done;

We only use the arg4/5/6/7 as anything other than 0 here,
so I think it would be clearer to write
   if (!ret) {
       usb_mtp_queue_result(s, RES_OK, d->trans, 3,
                            QEMU_STORAGE_ID, s->dataset.parent_handle,
                            handle);
       goto close;
  }

and then your code at the 'done' label can just unconditionally
use 0,0,0,0.

thanks
-- PMM