On Tue, 12 Mar 2019 at 18:25, Bandan Das <b...@redhat.com> wrote:
>
>
> Spotted by Coverity: CID 1399414
>
> mtp delete allows the return status of delete succeeded,
> partial_delete or readonly - when none of the objects could be
> deleted. Give more meaningful names to return values of the
> delete function.
>
> Some initiators recurse over the objects themselves. In that case,
> only READ_ONLY can be returned.
>
> Signed-off-by: Bandan Das <b...@redhat.com>
> ---
> v2:
> Change the enum variable names and specify them as bits
> Add a comment describing the bit definitions
> Modify commit message slightly
>
> hw/usb/dev-mtp.c | 62 ++++++++++++++++++++++++++----------------------
> 1 file changed, 34 insertions(+), 28 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 06e376bcd2..2b6fe77a6b 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1135,11 +1135,19 @@ static MTPData
> *usb_mtp_get_object_prop_value(MTPState *s, MTPControl *c,
> return d;
> }
>
> -/* Return correct return code for a delete event */
> +/*
> + * Return values when object @o is deleted.
> + * If at least one of the deletions succeeded,
> + * DELETE_SUCCESS is set and if at least one
> + * of the deletions failed, DELETE_FAILURE is
> + * set. Both bits being set (DELETE_PARTIAL)
> + * signifies a RES_PARITAL_DELETE being sent
"PARTIAL"
> + * back to the initiator.
> + */
> + switch (ret) {
> + case DELETE_SUCCESS:
> usb_mtp_queue_result(s, RES_OK, trans,
> 0, 0, 0, 0);
> - return;
> + break;
> + case DELETE_FAILURE:
> + usb_mtp_queue_result(s, RES_STORE_READ_ONLY,
> + trans, 0, 0, 0, 0);
> + break;
> + case DELETE_PARTIAL:
> + usb_mtp_queue_result(s, RES_PARTIAL_DELETE,
> + trans, 0, 0, 0, 0);
> + break;
> + default:
> + assert(ret != 0);
You could just write the default case as:
default:
g_assert_not_reached();
I think ?
> }
Otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
