On Wed, 30 Jan 2019 at 07:41, Gerd Hoffmann <[email protected]> wrote:
>
> From: Bandan Das <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Bandan Das <[email protected]>
> Message-id: [email protected]
> Signed-off-by: Gerd Hoffmann <[email protected]>
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
