qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 01/18] qapi: Smarter camel_to_upper() to reduce need for 'pre


From: Avihai Horon
Subject: Re: [PATCH 01/18] qapi: Smarter camel_to_upper() to reduce need for 'prefix'
Date: Tue, 30 Jul 2024 16:33:31 +0300
User-agent: Mozilla Thunderbird


On 30/07/2024 15:22, Markus Armbruster wrote:
External email: Use caution opening links or attachments


Avihai, there's a question for you on VfioMigrationState.

Daniel P. Berrangé <berrange@redhat.com> writes:

On Tue, Jul 30, 2024 at 10:10:15AM +0200, Markus Armbruster wrote:
camel_to_upper() converts its argument from camel case to upper case
with '_' between words.  Used for generated enumeration constant
prefixes.

When some of the words are spelled all caps, where exactly to insert
'_' is guesswork.  camel_to_upper()'s guesses are bad enough in places
to make people override them with a 'prefix' in the schema.

Rewrite it to guess better:

1. Insert '_' after a non-upper case character followed by an upper
    case character:

        OneTwo -> ONE_TWO
        One2Three -> ONE2_THREE

2. Insert '_' before the last upper case character followed by a
    non-upper case character:

        ACRONYMWord -> ACRONYM_Word

    Except at the beginning (as in OneTwo above), or when there is
    already one:

        AbCd -> AB_CD

This changes the default enumeration constant prefix for a number of
enums.  Generated enumeration constants change only where the default
is not overridden with 'prefix'.

The following enumerations without a 'prefix' change:

     enum                                 old camel_to_upper()
                                  new camel_to_upper()
     ------------------------------------------------------------------
     DisplayGLMode                   DISPLAYGL_MODE
                                  DISPLAY_GL_MODE
     EbpfProgramID                   EBPF_PROGRAMID
                                  EBPF_PROGRAM_ID
     HmatLBDataType                  HMATLB_DATA_TYPE
                                  HMAT_LB_DATA_TYPE
     HmatLBMemoryHierarchy           HMATLB_MEMORY_HIERARCHY
                                  HMAT_LB_MEMORY_HIERARCHY
     MultiFDCompression              MULTIFD_COMPRESSION
                                  MULTI_FD_COMPRESSION
     OffAutoPCIBAR                   OFF_AUTOPCIBAR
                                  OFF_AUTO_PCIBAR
     QCryptoBlockFormat              Q_CRYPTO_BLOCK_FORMAT
                                  QCRYPTO_BLOCK_FORMAT
     QCryptoBlockLUKSKeyslotState    Q_CRYPTO_BLOCKLUKS_KEYSLOT_STATE
                                  QCRYPTO_BLOCK_LUKS_KEYSLOT_STATE
     QKeyCode                        Q_KEY_CODE
                                  QKEY_CODE
     XDbgBlockGraphNodeType          X_DBG_BLOCK_GRAPH_NODE_TYPE
                                  XDBG_BLOCK_GRAPH_NODE_TYPE
     TestUnionEnumA               TEST_UNION_ENUMA
                                  TEST_UNION_ENUM_A

Add a 'prefix' so generated code doesn't change now.  Subsequent
commits will remove most of them again.  Two will remain:
MULTIFD_COMPRESSION, because migration code generally spells "multifd"
that way, and Q_KEY_CODE, because that one is baked into
subprojects/keycodemapdb/tools/keymap-gen.

The following enumerations with a 'prefix' change so that the prefix
is now superfluous:

     enum                                 old camel_to_upper()
                                  new camel_to_upper() [equal to prefix]
     ------------------------------------------------------------------
     BlkdebugIOType                  BLKDEBUGIO_TYPE
                                  BLKDEBUG_IO_TYPE
     QCryptoTLSCredsEndpoint         Q_CRYPTOTLS_CREDS_ENDPOINT
                                  QCRYPTO_TLS_CREDS_ENDPOINT
     QCryptoSecretFormat             Q_CRYPTO_SECRET_FORMAT
                                  QCRYPTO_SECRET_FORMAT
     QCryptoCipherMode               Q_CRYPTO_CIPHER_MODE
                                  QCRYPTO_CIPHER_MODE
     QCryptodevBackendType           Q_CRYPTODEV_BACKEND_TYPE
                                  QCRYPTODEV_BACKEND_TYPE
     QType [builtin]                 Q_TYPE
                                  QTYPE

Drop these prefixes.

The following enumerations with a 'prefix' change without making the
'prefix' superfluous:

     enum                                 old camel_to_upper()
                                  new camel_to_upper() [equal to prefix]
                                  prefix
     ------------------------------------------------------------------
     CpuS390Entitlement              CPUS390_ENTITLEMENT
                                  CPU_S390_ENTITLEMENT
                                  S390_CPU_ENTITLEMENT
     CpuS390Polarization             CPUS390_POLARIZATION
                                  CPU_S390_POLARIZATION
                                  S390_CPU_POLARIZATION
     CpuS390State                    CPUS390_STATE
                                  CPU_S390_STATE
                                  S390_CPU_STATE
     QAuthZListFormat                Q_AUTHZ_LIST_FORMAT
                                  QAUTH_Z_LIST_FORMAT
                                  QAUTHZ_LIST_FORMAT
     QAuthZListPolicy                Q_AUTHZ_LIST_POLICY
                                  QAUTH_Z_LIST_POLICY
                                  QAUTHZ_LIST_POLICY
     QCryptoAkCipherAlgorithm        Q_CRYPTO_AK_CIPHER_ALGORITHM
                                  QCRYPTO_AK_CIPHER_ALGORITHM
                                  QCRYPTO_AKCIPHER_ALG
     QCryptoAkCipherKeyType          Q_CRYPTO_AK_CIPHER_KEY_TYPE
                                  QCRYPTO_AK_CIPHER_KEY_TYPE
                                  QCRYPTO_AKCIPHER_KEY_TYPE
     QCryptoCipherAlgorithm          Q_CRYPTO_CIPHER_ALGORITHM
                                  QCRYPTO_CIPHER_ALGORITHM
                                  QCRYPTO_CIPHER_ALG
     QCryptoHashAlgorithm            Q_CRYPTO_HASH_ALGORITHM
                                  QCRYPTO_HASH_ALGORITHM
                                  QCRYPTO_HASH_ALG
     QCryptoIVGenAlgorithm           Q_CRYPTOIV_GEN_ALGORITHM
                                  QCRYPTO_IV_GEN_ALGORITHM
                                  QCRYPTO_IVGEN_ALG
     QCryptoRSAPaddingAlgorithm      Q_CRYPTORSA_PADDING_ALGORITHM
                                  QCRYPTO_RSA_PADDING_ALGORITHM
                                  QCRYPTO_RSA_PADDING_ALG
     QCryptodevBackendAlgType        Q_CRYPTODEV_BACKEND_ALG_TYPE
                                  QCRYPTODEV_BACKEND_ALG_TYPE
                                  QCRYPTODEV_BACKEND_ALG
     QCryptodevBackendServiceType    Q_CRYPTODEV_BACKEND_SERVICE_TYPE
                                  QCRYPTODEV_BACKEND_SERVICE_TYPE
                                  QCRYPTODEV_BACKEND_SERVICE

Subsequent commits will tweak things to remove most of these prefixes.
Only QAUTHZ_LIST_FORMAT and QAUTHZ_LIST_POLICY will remain.
IIUC from above those two result in

                           QAUTH_Z_LIST_FORMAT
                           QAUTH_Z_LIST_POLICY

Is it possible to add a 3rd rule

  *  Single uppercase letter folds into the previous word
I guess we could.

or are there valid cases where we have a single uppercase
that we want to preserve ?
Not now, but I'd prefer to leave predictions to economists.

It sure would be nice to eliminate the 'prefix' concept,
that we've clearly over-used, if we can kill the only 2
remaining examples.
There are a few more, actually.  After this series and outside tests:

     enum                            default prefix camel_to_upper()
                                     prefix override
     ------------------------------------------------------------------
     BlkdebugEvent                   BLKDEBUG_EVENT
                                     BLKDBG
     IscsiHeaderDigest               ISCSI_HEADER_DIGEST
                                     QAPI_ISCSI_HEADER_DIGEST
     MultiFDCompression              MULTI_FD_COMPRESSION
                                     MULTIFD_COMPRESSION
     QAuthZListFormat                QAUTH_Z_LIST_FORMAT
                                     QAUTHZ_LIST_FORMAT
     QAuthZListPolicy                QAUTH_Z_LIST_POLICY
                                     QAUTHZ_LIST_POLICY
     QKeyCode                        QKEY_CODE
                                     Q_KEY_CODE
     VfioMigrationState              VFIO_MIGRATION_STATE
                                     QAPI_VFIO_MIGRATION_STATE

Reasons for 'prefix', and what could be done instead of 'prefix':

* BlkdebugEvent: shorten the prefix.

   Could live with the longer names instead.  Some 90 occurences...

* IscsiHeaderDigest

   QAPI version of enum iscsi_header_digest from libiscsi's
   iscsi/iscsi.h.  We use 'prefix' to avoid name clashes.

   Could rename the type to QapiIscsiHeaderDigest instead.

* MultiFDCompression

   Migration code consistently uses prefixes multifd_, MULTIFD_, and
   MultiFD_.

   Could rename the type to MultifdCompression instead, but that just
   moves the inconsistency to the type name.

* QAuthZListFormat and QAuthZListPolicy

   The authz code consistently uses QAuthZ.

   Could make camel_to_upper() avoid the lone Z instead (and hope that'll
   remain what we want).

* QKeyCode

   Q_KEY_CODE is baked into subprojects/keycodemapdb/tools/keymap-gen.

   Could adjust the subproject instead.

* VfioMigrationState

   Can't see why this one has a prefix.  Avihai, can you enlighten me?

linux-headers/linux/vfio.h defines enum vfio_device_mig_state with values VFIO_DEVICE_STATE_STOP etc. I used the QAPI prefix to emphasize this is a QAPI entity rather than a VFIO entity.

Thanks.


Daniel, thoughts?

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
  qapi/block-core.json                     |  3 +-
  qapi/common.json                         |  1 +
  qapi/crypto.json                         |  6 ++--
  qapi/cryptodev.json                      |  1 -
  qapi/ebpf.json                           |  1 +
  qapi/machine.json                        |  1 +
  qapi/migration.json                      |  1 +
  qapi/ui.json                             |  2 ++
  scripts/qapi/common.py                   | 42 ++++++++++++++----------
  scripts/qapi/schema.py                   |  2 +-
  tests/qapi-schema/alternate-array.out    |  1 -
  tests/qapi-schema/comments.out           |  1 -
  tests/qapi-schema/doc-good.out           |  1 -
  tests/qapi-schema/empty.out              |  1 -
  tests/qapi-schema/include-repetition.out |  1 -
  tests/qapi-schema/include-simple.out     |  1 -
  tests/qapi-schema/indented-expr.out      |  1 -
  tests/qapi-schema/qapi-schema-test.json  |  1 +
  tests/qapi-schema/qapi-schema-test.out   |  2 +-
  19 files changed, 37 insertions(+), 33 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Thanks!




reply via email to

[Prev in Thread] Current Thread [Next in Thread]