Re: [Qemu-devel] [PULL 3/4] usb-mtp: prevent null dereference while deleting objects

[Peter Maydell]

On Thu, 7 Mar 2019 at 09:56, Gerd Hoffmann <address@hidden> wrote:
>
> From: Bandan Das <address@hidden>
>
> Spotted by Coverity: CID 1399144
>
> Signed-off-by: Bandan Das <address@hidden>
> Message-id: address@hidden
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  hw/usb/dev-mtp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 1f22284949df..06e376bcd211 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1177,9 +1177,7 @@ static int usb_mtp_deletefn(MTPState *s, MTPObject *o, 
> uint32_t trans)
>              usb_mtp_object_free_one(s, o);
>              success = true;
>          }
> -    }
> -
> -    if (o->format == FMT_ASSOCIATION) {
> +    } else if (o->format == FMT_ASSOCIATION) {
>          if (rmdir(o->path)) {
>              partial_delete = true;
>          } else {
> --

Hi; following this change Coverity now complains (CID 1399414)
about dead code later in the file:

In this set of if/else clauses, either we
set partial_delete to true, or we set success to
true, but never both:

    if (o->format == FMT_UNDEFINED_OBJECT) {
        if (remove(o->path)) {
            partial_delete = true;
        } else {
            usb_mtp_object_free_one(s, o);
            success = true;
        }
    } else if (o->format == FMT_ASSOCIATION) {
        if (rmdir(o->path)) {
            partial_delete = true;
        } else {
            usb_mtp_object_free_one(s, o);
            success = true;
        }
    }

and so here:

    if (success && partial_delete) {
        return PARTIAL_DELETE;
    }

the condition can never be true and the code inside
the if () {} is dead.

When is the routine intended to return the PARTIAL_DELETE
return value ?

thanks
-- PMM