Re: [Qemu-devel] [PULL v2 01/16] qemu-io: add pattern file for write command

[Peter Maydell]

On Tue, 3 Sep 2019 at 14:35, Max Reitz <address@hidden> wrote:
>
> From: Denis Plotnikov <address@hidden>
>
> The patch allows to provide a pattern file for write
> command. There was no similar ability before.
>
> Signed-off-by: Denis Plotnikov <address@hidden>
> Message-id: address@hidden
> Reviewed-by: Eric Blake <address@hidden>
> [mreitz: Keep optstring in alphabetical order]
> Signed-off-by: Max Reitz <address@hidden>

Hi; Coverity finds a FILE* leak in this code (CID 1405303):



> +/*
> + * qemu_io_alloc_from_file()
> + *
> + * Allocates the buffer and populates it with the content of the given file
> + * up to @len bytes. If the file length is less than @len, then the buffer
> + * is populated with the file content cyclically.
> + *
> + * @blk - the block backend where the buffer content is going to be written 
> to
> + * @len - the buffer length
> + * @file_name - the file to read the content from
> + *
> + * Returns: the buffer pointer on success
> + *          NULL on error
> + */
> +static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,
> +                                     const char *file_name)
> +{
> +    char *buf, *buf_origin;
> +    FILE *f = fopen(file_name, "r");

Here we allocate the FILE*...

> +    int pattern_len;
> +
> +    if (!f) {
> +        perror(file_name);
> +        return NULL;
> +    }
> +
> +    if (qemuio_misalign) {
> +        len += MISALIGN_OFFSET;
> +    }
> +
> +    buf_origin = buf = blk_blockalign(blk, len);
> +
> +    if (qemuio_misalign) {
> +        buf_origin += MISALIGN_OFFSET;
> +        buf += MISALIGN_OFFSET;
> +        len -= MISALIGN_OFFSET;
> +    }
> +
> +    pattern_len = fread(buf_origin, 1, len, f);
> +
> +    if (ferror(f)) {
> +        perror(file_name);
> +        goto error;

...but in this error-exit path...

> +    }
> +
> +    if (pattern_len == 0) {
> +        fprintf(stderr, "%s: file is empty\n", file_name);
> +        goto error;

...and this one...

> +    }
> +
> +    fclose(f);
> +
> +    if (len > pattern_len) {
> +        len -= pattern_len;
> +        buf += pattern_len;
> +
> +        while (len > 0) {
> +            size_t len_to_copy = MIN(pattern_len, len);
> +
> +            memcpy(buf, buf_origin, len_to_copy);
> +
> +            len -= len_to_copy;
> +            buf += len_to_copy;
> +        }
> +    }
> +
> +    return buf_origin;
> +
> +error:
> +    qemu_io_free(buf_origin);
> +    return NULL;

...we go to the 'error' label and leave the function without
ever calling fclose(f).

> +}

thanks
-- PMM