Re: [Qemu-devel] [PATCH 1/2] qemu-img: Fix dd with skip= and count=

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

On 2018-08-16 04:17, Eric Blake wrote:
> On 08/15/2018 09:03 PM, Max Reitz wrote:
> 
>>> @@ -4559,19 +4559,23 @@ static int img_dd(int argc, char **argv)
>>>           goto out;
>>>       }
>>>
>>> +    /* Overflow means the specified offset is beyond input image's
>>> size */
>>> +    if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
>>> +                              size < in.bsz * in.offset)) {
>>> +        size = 0;
>>> +        error_report("%s: cannot skip to specified offset",
>>> in.filename);
>>
>> in_pos should be initialized as well (to "size", I suppose), or my gcc
>> will continue to complain. :-)
>>
> 
> Serves me right for compiling with -g instead of -O2 (gcc only does that
> warning on optimized builds, for some weird reason).  Will fix in v2.
> 
> 
>>> +++ b/tests/qemu-iotests/160
>>> @@ -44,6 +44,7 @@ _supported_os Linux
>>>   TEST_SKIP_BLOCKS="1 2 30 30K"
>>>
>>>   for skip in $TEST_SKIP_BLOCKS; do
>>> +  for count in '' 'count=1 '; do
>>
>> Ah, so this is why we indent everything by four spaces!  So you can
>> squeeze in three more block headers without having to re-indent
>> everything.  I finally see. O:-)
> 
> I'm seriously thinking of reindenting things in this and the next patch,
> rather than my initial quick-and-dirty "squeeze it in".  'git diff -w'
> is not that hard to use, after all.
> 
>>
>> (Not sure why you put a space after the 'count=1', though, but OK)
> 
> For this line:
> 
> +    echo "== Converting the image with dd with ${count}skip=$skip =="
> 
> so that when $count is empty, the .out file doesn't end up with a double
> space.  Okay, I do have some sense of output aesthetics, even if my
> re-indentation skills are lacking ;=)

Ah, right, I forgot you'd get the double space.  That's true.

Max


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