[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: |
Markus Armbruster |
Subject: |
Re: [PATCH 01/18] qapi: Smarter camel_to_upper() to reduce need for 'prefix' |
Date: |
Tue, 30 Jul 2024 14:22:05 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
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?
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!
- [PATCH 09/18] qapi/machine: Rename CpuS390* to S390Cpu, and drop 'prefix', (continued)
- [PATCH 09/18] qapi/machine: Rename CpuS390* to S390Cpu, and drop 'prefix', Markus Armbruster, 2024/07/30
- [PATCH 08/18] qapi/ui: Drop temporary 'prefix', Markus Armbruster, 2024/07/30
- [PATCH 04/18] qapi/common: Drop temporary 'prefix', Markus Armbruster, 2024/07/30
- [PATCH 03/18] qapi/block-core: Drop temporary 'prefix', Markus Armbruster, 2024/07/30
- [PATCH 02/18] tests/qapi-schema: Drop temporary 'prefix', Markus Armbruster, 2024/07/30
- [PATCH 01/18] qapi: Smarter camel_to_upper() to reduce need for 'prefix', Markus Armbruster, 2024/07/30
Re: [PATCH 01/18] qapi: Smarter camel_to_upper() to reduce need for 'prefix', Kevin Wolf, 2024/07/31
[PATCH 07/18] qapi/machine: Drop temporary 'prefix', Markus Armbruster, 2024/07/30
[PATCH 15/18] qapi/crypto: Rename QCryptoRSAPaddingAlgorithm to *Algo, and drop prefix, Markus Armbruster, 2024/07/30
[PATCH 10/18] qapi/crypto: Drop unwanted 'prefix', Markus Armbruster, 2024/07/30