Re: [PATCH v3 1/4] luks: extract qcrypto_block_calculate_payload_offset()

2020-02-21 Thread Stefan Hajnoczi
On Wed, Feb 19, 2020 at 02:03:50PM +0100, Max Reitz wrote:
> On 11.02.20 17:03, Stefan Hajnoczi wrote:
> > The qcow2 .bdrv_measure() code calculates the crypto payload offset.
> > This logic really belongs in crypto/block.c where it can be reused by
> > other image formats.
> > 
> > The "luks" block driver will need this same logic in order to implement
> > .bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()
> > function now.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  block/qcow2.c  | 74 +++---
> >  crypto/block.c | 40 +++
> >  include/crypto/block.h | 22 +
> >  3 files changed, 81 insertions(+), 55 deletions(-)
> 
> [...]
> 
> > diff --git a/crypto/block.c b/crypto/block.c
> > index 325752871c..a9e1b8cc36 100644
> > --- a/crypto/block.c
> > +++ b/crypto/block.c
> > @@ -115,6 +115,46 @@ QCryptoBlock 
> > *qcrypto_block_create(QCryptoBlockCreateOptions *options,
> 
> [...]
> 
> > +bool
> > +qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions 
> > *create_opts,
> > +   const char *optprefix,
> > +   size_t *len,
> > +   Error **errp)
> > +{
> > +QCryptoBlock *crypto;
> > +bool ok;
> > +
> > +/* Fake LUKS creation in order to determine the payload size */
> > +crypto = qcrypto_block_create(create_opts, optprefix,
> > +  qcrypto_block_headerlen_hdr_init_func,
> > +  qcrypto_block_headerlen_hdr_write_func,
> > +  len, errp);
> > +ok = crypto != NULL;
> > +qcrypto_block_free(crypto);
> > +return ok;
> 
> Speaking of g_autoptr...  Would g_autoptr(QCryptoBlock) crypto; suffice
> to contract these three lines into “return crypto != NULL;”?
> 
> Either way:
> 
> Reviewed-by: Max Reitz 

Yes, it can be simplified.  Will fix in v3.

Stefan


signature.asc
Description: PGP signature


Re: [PATCH v3 1/4] luks: extract qcrypto_block_calculate_payload_offset()

2020-02-19 Thread Max Reitz
On 11.02.20 17:03, Stefan Hajnoczi wrote:
> The qcow2 .bdrv_measure() code calculates the crypto payload offset.
> This logic really belongs in crypto/block.c where it can be reused by
> other image formats.
> 
> The "luks" block driver will need this same logic in order to implement
> .bdrv_measure(), so extract the qcrypto_block_calculate_payload_offset()
> function now.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  block/qcow2.c  | 74 +++---
>  crypto/block.c | 40 +++
>  include/crypto/block.h | 22 +
>  3 files changed, 81 insertions(+), 55 deletions(-)

[...]

> diff --git a/crypto/block.c b/crypto/block.c
> index 325752871c..a9e1b8cc36 100644
> --- a/crypto/block.c
> +++ b/crypto/block.c
> @@ -115,6 +115,46 @@ QCryptoBlock 
> *qcrypto_block_create(QCryptoBlockCreateOptions *options,

[...]

> +bool
> +qcrypto_block_calculate_payload_offset(QCryptoBlockCreateOptions 
> *create_opts,
> +   const char *optprefix,
> +   size_t *len,
> +   Error **errp)
> +{
> +QCryptoBlock *crypto;
> +bool ok;
> +
> +/* Fake LUKS creation in order to determine the payload size */
> +crypto = qcrypto_block_create(create_opts, optprefix,
> +  qcrypto_block_headerlen_hdr_init_func,
> +  qcrypto_block_headerlen_hdr_write_func,
> +  len, errp);
> +ok = crypto != NULL;
> +qcrypto_block_free(crypto);
> +return ok;

Speaking of g_autoptr...  Would g_autoptr(QCryptoBlock) crypto; suffice
to contract these three lines into “return crypto != NULL;”?

Either way:

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature