Re: [PATCH 11/18] qapi/crypto: Rename QCryptoHashAlgorithm to *Algo, and drop prefix

[Daniel P . Berrangé]

On Tue, Jul 30, 2024 at 02:26:49PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Jul 30, 2024 at 10:10:25AM +0200, Markus Armbruster wrote:
> >> QAPI's 'prefix' feature can make the connection between enumeration
> >> type and its constants less than obvious.  It's best used with
> >> restraint.
> >> 
> >> QCryptoHashAlgorithm has a 'prefix' that overrides the generated
> >> enumeration constants' prefix to QCRYPTO_HASH_ALG.
> >> 
> >> We could simply drop 'prefix', but then the prefix becomes
> >> QCRYPTO_HASH_ALGORITHM, which is rather long.
> >> 
> >> We could additionally rename the type to QCryptoHashAlg, but I think
> >> the abbreviation "alg" is less than clear.
> >
> > I would have gone with this, but it is a bit of a bike shed colouring
> > debate so I'm not fussed
> 
> Either solution seems okay, so I went with my personal preference.  Do
> feel free to state yours and ask me to respin!

After reviewing the patches that follow, I'd observe that picking
Algo has made the following patches much larger than if it had
stuck with Alg. Basically changing both the types & constants,
instead of only having to change the types. 

> 
> >> Rename the type to QCryptoHashAlgo instead.  The prefix becomes to
> >> QCRYPTO_HASH_ALGO.
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  qapi/crypto.json                        | 17 +++++-----
> >>  crypto/blockpriv.h                      |  2 +-
> >>  crypto/hashpriv.h                       |  2 +-
> >>  crypto/hmacpriv.h                       |  4 +--
> >>  crypto/ivgenpriv.h                      |  2 +-
> >>  include/crypto/afsplit.h                |  8 ++---
> >>  include/crypto/block.h                  |  2 +-
> >>  include/crypto/hash.h                   | 18 +++++-----
> >>  include/crypto/hmac.h                   |  6 ++--
> >>  include/crypto/ivgen.h                  |  6 ++--
> >>  include/crypto/pbkdf.h                  | 10 +++---
> >>  backends/cryptodev-builtin.c            |  8 ++---
> >>  backends/cryptodev-lkcf.c               | 10 +++---
> >>  block/parallels-ext.c                   |  2 +-
> >>  block/quorum.c                          |  4 +--
> >>  crypto/afsplit.c                        |  6 ++--
> >>  crypto/block-luks.c                     | 16 ++++-----
> >>  crypto/block.c                          |  2 +-
> >>  crypto/hash-afalg.c                     | 26 +++++++--------
> >>  crypto/hash-gcrypt.c                    | 20 +++++------
> >>  crypto/hash-glib.c                      | 20 +++++------
> >>  crypto/hash-gnutls.c                    | 20 +++++------
> >>  crypto/hash-nettle.c                    | 18 +++++-----
> >>  crypto/hash.c                           | 30 ++++++++---------
> >>  crypto/hmac-gcrypt.c                    | 22 ++++++-------
> >>  crypto/hmac-glib.c                      | 22 ++++++-------
> >>  crypto/hmac-gnutls.c                    | 22 ++++++-------
> >>  crypto/hmac-nettle.c                    | 22 ++++++-------
> >>  crypto/hmac.c                           |  2 +-
> >>  crypto/ivgen.c                          |  4 +--
> >>  crypto/pbkdf-gcrypt.c                   | 36 ++++++++++----------
> >>  crypto/pbkdf-gnutls.c                   | 36 ++++++++++----------
> >>  crypto/pbkdf-nettle.c                   | 32 +++++++++---------
> >>  crypto/pbkdf-stub.c                     |  4 +--
> >>  crypto/pbkdf.c                          |  2 +-
> >>  hw/misc/aspeed_hace.c                   | 16 ++++-----
> >>  io/channel-websock.c                    |  2 +-
> >>  target/i386/sev.c                       |  6 ++--
> >>  tests/bench/benchmark-crypto-akcipher.c | 12 +++----
> >>  tests/bench/benchmark-crypto-hash.c     | 10 +++---
> >>  tests/bench/benchmark-crypto-hmac.c     |  6 ++--
> >>  tests/unit/test-crypto-afsplit.c        | 10 +++---
> >>  tests/unit/test-crypto-akcipher.c       |  6 ++--
> >>  tests/unit/test-crypto-block.c          | 16 ++++-----
> >>  tests/unit/test-crypto-hash.c           | 42 +++++++++++------------
> >>  tests/unit/test-crypto-hmac.c           | 16 ++++-----
> >>  tests/unit/test-crypto-ivgen.c          |  8 ++---
> >>  tests/unit/test-crypto-pbkdf.c          | 44 ++++++++++++-------------
> >>  ui/vnc.c                                |  2 +-
> >>  util/hbitmap.c                          |  2 +-
> >>  crypto/akcipher-gcrypt.c.inc            | 14 ++++----
> >>  crypto/akcipher-nettle.c.inc            | 26 +++++++--------
> >>  52 files changed, 350 insertions(+), 351 deletions(-)
> >
> > Acked-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Thanks!
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|