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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2949 bytes --]

On 09.09.19 19:26, Peter Maydell wrote:
> On Tue, 3 Sep 2019 at 14:35, Max Reitz <mreitz@redhat.com> wrote:
>>
>> From: Denis Plotnikov <dplotnikov@virtuozzo.com>
>>
>> The patch allows to provide a pattern file for write
>> command. There was no similar ability before.
>>
>> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>> Message-id: 20190820164616.4072-1-dplotnikov@virtuozzo.com
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> [mreitz: Keep optstring in alphabetical order]
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Hi; Coverity finds a FILE* leak in this code (CID 1405303):

Thanks for the heads-up.  Denis, do you want to write a patch?

Max

>> +/*
>> + * 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
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]