Re: [Qemu-devel] [PATCH 03/13] qcrypto-luks: refactoring: extract load/store/check/parse header functions

[Maxim Levitsky]

On Thu, 2019-08-22 at 11:34 +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 22, 2019 at 01:43:05AM +0300, Maxim Levitsky wrote:
> > On Tue, 2019-08-20 at 20:01 +0200, Max Reitz wrote:
> > > On 14.08.19 22:22, Maxim Levitsky wrote:
> > > > With upcoming key management, the header will
> > > > need to be stored after the image is created.
> > > > 
> > > > Extracting load header isn't strictly needed, but
> > > > do this anyway for the symmetry.
> > > > 
> > > > Also I extracted a function that does basic sanity
> > > > checks on the just read header, and a function
> > > > which parses all the crypto format to make the
> > > > code a bit more readable, plus now the code
> > > > doesn't destruct the in-header cipher-mode string,
> > > > so that the header now can be stored many times,
> > > > which is needed for the key management.
> > > > 
> > > > Also this allows to contain the endianess conversions
> > > > in these functions alone
> > > > 
> > > > The header is no longer endian swapped in place,
> > > > to prevent (mostly theoretical races I think)
> > > > races where someone could see the header in the
> > > > process of beeing byteswapped.
> > > 
> > > The formatting looks weird, it doesn’t look quite 72 characters wide...
> > >  (what commit messages normally use)
> > 
> > Could you elaborate on that? I thought that code should not
> > exceed 80 character limit.
> > 
> > > 
> > > > Signed-off-by: Maxim Levitsky <address@hidden>
> > > > ---
> > > >  crypto/block-luks.c | 756 ++++++++++++++++++++++++++------------------
> > > >  1 file changed, 440 insertions(+), 316 deletions(-)
> > > 
> > > Also, this commit is just too big.
> > 
> > Yea, but it has no functional changes.
> > I can split it further, but that won't help much IMHO.
> 
> I'd find it easier to review if each newly introduced method was a
> separate patch. It makes it easier to see which bit of removed
> code was added to which method.
> 
> Regards,
> Daniel

Done, patch is now split in several patches.
Best regards,
        Maxim Levitsky