On Tue, 19 Mar 2019 at 21:46, Bandan Das <b...@redhat.com> 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 <b...@redhat.com>
> ---
> 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
