On Wed, 30 Jan 2019 at 07:41, Gerd Hoffmann <kra...@redhat.com> wrote: > > From: Bandan Das <b...@redhat.com> > > For every MTP_WRITE_BUF_SZ copied, this patch writes it to file before > getting the next block of data. The file is kept opened for the > duration of the operation but the sanity checks on the write operation > are performed only once when the write operation starts. Additionally, > we also update the file size in the object metadata once the file has > completely been written. > > Suggested-by: Gerd Hoffman <kra...@redhat.com> > Signed-off-by: Bandan Das <b...@redhat.com> > Message-id: 20190129131908.27924-3-...@redhat.com > Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
Hi; Coverity has spotted a couple of issues with this patch: > +static void usb_mtp_update_object(MTPObject *parent, char *name) > +{ > + MTPObject *o = > + usb_mtp_object_lookup_name(parent, name, strlen(name)); > + > + if (o) { > + lstat(o->path, &o->stat); CID 1398651: We don't check the return value of this lstat() for failure. > + } > +} > + > static void usb_mtp_write_data(MTPState *s) > { > MTPData *d = s->data_out; [...] > + case WRITE_CONTINUE: > + case WRITE_END: > + 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); > goto done; > + } > + if (d->write_status != WRITE_END) { > + return; CID 1398642: This early-return case in usb_mtp_write_data() returns from the function without doing any of the cleanup (closing file, freeing data, etc). Possibly it should be "goto done;" instead ? The specific thing Coverity complains about is the memory pointed to by "path". thanks -- PMM