On 31.05.2019 10:14, Stefano Garzarella wrote:
> On Fri, May 31, 2019 at 06:21:13AM +0000, Denis Plotnikov wrote:
>>
>>
>> On 30.05.2019 11:26, Stefano Garzarella wrote:
>>> On Thu, May 30, 2019 at 11:10:55AM +0300, Denis Plotnikov wrote:
>>>> The patch allows to provide a pattern file for write
>>>> command. There was no similar ability before.
>>>>
>>>> Signed-off-by: Denis Plotnikov <dplotni...@virtuozzo.com>
>>>> ---
>>>
>>> Hi Denis,
>>> for next versions I suggest to describe here, after the ---, the changes
>>> with previous versions. In this way the review should be easier.
>> ---
>> Sure
>>>
>>>> qemu-io-cmds.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++----
>>>> 1 file changed, 75 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>>>> index 09750a23ce..1c6a5e4faf 100644
>>>> --- a/qemu-io-cmds.c
>>>> +++ b/qemu-io-cmds.c
>>>> @@ -21,6 +21,7 @@
>>>> #include "qemu/option.h"
>>>> #include "qemu/timer.h"
>>>> #include "qemu/cutils.h"
>>>> +#include "string.h"
>>>>
>>>> #define CMD_NOFILE_OK 0x01
>>>>
>>>> @@ -343,6 +344,61 @@ static void *qemu_io_alloc(BlockBackend *blk, size_t
>>>> len, int pattern)
>>>> return buf;
>>>> }
>>>>
>>>> +static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,
>>>> + char *file_name)
>>>> +{
>>>> + char *buf, *buf_pos;
>>>> + FILE *f = fopen(file_name, "r");
>>>> + int l;
>>>> +
>>>> + if (!f) {
>>>> + printf("'%s': %s\n", file_name, strerror(errno));
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + if (qemuio_misalign) {
>>>> + len += MISALIGN_OFFSET;
>>>> + }
>>>> + buf = blk_blockalign(blk, len);
>>>> + memset(buf, 0, len);
>>>> +
>>>> + buf_pos = buf;
>>>> +
>>>> + while (len > 0) {
>>>> + l = fread(buf_pos, sizeof(char), len, f);
>>>> +
>>>> + if (feof(f)) {
>>>> + rewind(f);
>>>> + }
>>>> +
>>>> + if (ferror(f)) {
>>>> + printf("'%s': %s\n", file_name, strerror(errno));
>>>> + goto error;
>>>> + }
>>>> +
>>>> + if (l == 0) {
>>>> + printf("'%s' is empty\n", file_name);
>>>> + goto error;
>>>> + }
>>>
>>> Could it happen that we read some bytes, than we reached the EOF, so we made
>>> the rewind() and fread() returns 0?
>>> If that can happen, maybe we should change it in this way:
>>>
>>> if (l == 0 && buf_pos == buf) {
>>> printf("'%s' is empty\n", file_name);
>>> goto error;
>>> }
>> This shouldn't happen. To get that situation we need to read exactly the
>> file length (from the current position to the end) on the first step,
>> then rewind, read again and in that case get l == 0 and feof == true.
>> But reading the exact length means that we asked for that size, so the
>> buffer is fully populated, thus len == 0 and we leave the loop.
>>
>> So, l == 0 is only when we read an empty file on the first iteration.
>>
>
> Right!
>
>>>> @@ -983,8 +1039,9 @@ static int write_f(BlockBackend *blk, int argc, char
>>>> **argv)
>>>> /* Some compilers get confused and warn if this is not initialized.
>>>> */
>>>> int64_t total = 0;
>>>> int pattern = 0xcd;
>>>> + char *file_name;
>
> I think we should initialize file_name to NULL here to silent the compiler.
> I applied this patch and I had this error:
> /home/stefano/repos/qemu/qemu-io-cmds.c: In function ‘write_f’:
> /home/stefano/repos/qemu/qemu-io-cmds.c:351:15: error: ‘file_name’ may be
> used uninitialized in this function [-Werror=maybe-uninitialized]
> FILE *f = fopen(file_name, "r");
> ^~~~~~~~~~~~~~~~~~~~~
> /home/stefano/repos/qemu/qemu-io-cmds.c:1042:11: note: ‘file_name’ was
> declared here
> char *file_name;
> ^~~~~~~~~
> cc1: all warnings being treated as errors
> make: *** [/home/stefano/repos/qemu/rules.mak:69: qemu-io-cmds.o] Error 1
> make: *** Waiting for unfinished jobs....
ok, will do
>
> Thanks,
> Stefano
>
--
Best,
Denis
