Re: [Qemu-devel] [PATCH] block: vhdx - fix reading beyond pointer during image creation

[Markus Armbruster <armbru@redhat.com>]

Markus Armbruster <armbru@redhat.com> writes:

> Jeff Cody <jcody@redhat.com> writes:
>
>> On Wed, Sep 17, 2014 at 08:33:10AM +0200, Markus Armbruster wrote:
>>> Jeff Cody <jcody@redhat.com> writes:
>>> 
>>> > In vhdx_create_metadata(), we allocate 40 bytes to entry_buffer for
>>> > the various metadata table entries.  However, we write out 64kB from
>>> > that buffer into the new file.  Only write out the correct 40 bytes.
>>> >
>>> > Signed-off-by: Jeff Cody <jcody@redhat.com>
>>> > ---
>>> >  block/vhdx.c | 16 ++++++++--------
>>> >  1 file changed, 8 insertions(+), 8 deletions(-)
>>> >
>>> > diff --git a/block/vhdx.c b/block/vhdx.c
>>> > index 796b7bd..b52ec32 100644
>>> > --- a/block/vhdx.c
>>> > +++ b/block/vhdx.c
>>> > @@ -1407,6 +1407,12 @@ exit:
>>> >      return ret;
>>> >  }
>>> >  
>>> > +#define VHDX_METADATA_ENTRY_BUFFER_SIZE \
>>> > +                                    (sizeof(VHDXFileParameters)               +\
>>> > +                                     sizeof(VHDXVirtualDiskSize)              +\
>>> > +                                     sizeof(VHDXPage83Data)                   +\
>>> > +                                     sizeof(VHDXVirtualDiskLogicalSectorSize) +\
>>> > +                                     sizeof(VHDXVirtualDiskPhysicalSectorSize))
>>> 
>>> Long lines, caused by excessive indentation.  Emacs suggests
>>> 
>>> #define VHDX_METADATA_ENTRY_BUFFER_SIZE         \
>>>     (sizeof(VHDXFileParameters)               + \
>>>      sizeof(VHDXVirtualDiskSize)              + \
>>>      sizeof(VHDXPage83Data)                   + \
>>>      sizeof(VHDXVirtualDiskLogicalSectorSize) + \
>>>      sizeof(VHDXVirtualDiskPhysicalSectorSize))
>>> 
>>
>> So, I was getting ready to respin this, but double checked the patch -
>> it shows the lines ending on column 80 (as intended), and
>> checkpatch.pl had no issue with it.  Did you accidentally (or
>> intentionally!) count the leading '+' of the patch itself?
>
> I didn't count anything, I trusted my eyes, which screamed "ugly!" :)

Forgot to say: since checkpatch is happy, I guess this is a technically
a matter of taste, so

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Just in case you actually *like* hanging your code right off the right
margin of the window.  *Shudder*  ;-P