Re: [RFC PATCH] qom/object: Disallow comma in type names

[Markus Armbruster <armbru@redhat.com>]

Thomas Huth <thuth@redhat.com> writes:

> Forgot to CC: qemu-devel (sorry) - thanks to Markus for the hint.
> So let's repeat it here:
>
> On 14/11/2023 10.31, Daniel P. Berrangé wrote:
>> On Tue, Nov 14, 2023 at 10:05:37AM +0100, Thomas Huth wrote:
>>> QOM names currently don't have any enforced naming rules. This can
>>> be problematic, e.g. when they are used on the command line for
>>> the "-device" option (where the comma is used to separate properties).
>>> To avoid that such problematic type names come in again, let's
>>> disallow them now by adding an g_assert() during the type registration.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   Based-on: <20231113134344.1195478-1-armbru@redhat.com>
>>>   (without Markus' patches, the g_assert() triggers with the current
>>>    code base)
>>>   
>>>   See discussion here:
>>>   https://lore.kernel.org/qemu-devel/87y1f0hjdh.fsf@pond.sub.org/
>>>
>>>   Questions: Should we disallow other characters, too? Slash and
>>>   backslash maybe (since they can cause trouble with module names)?
>>>   Dot and colon would maybe be good candidates, too, but they seem
>>>   to be in wide use already, so these don't really seem to be
>>>   feasible...
>> 
>> There's two questions.
>> 
>>    * What should we enforce today
>>    * What should we ideally enforce in future
>> 
>> Ideally the answers would be the same, but getting there will
>> almost certainly require some cleanup first.
>> 
>> Given that we can now define QOM types using QAPI, I feel we
>> preserve everyone's sanity by enforcing the same rules for
>> QOM and QAPI type naming. IOW

Agree!

>>    All QOM type names must begin with a letter, and contain
>>    only ASCII letters, digits, hyphen, and underscore.
>> 
>> is the answer for the second question.

As long as type names only occur as *values*, the next sentence's first
exception applies, too:

      There are two exceptions: enum values may start with a digit, and
      names that are downstream extensions (see section `Downstream
      extensions`_) start with underscore.

This is of course docs/devel/qapi-code-gen.rst.

I'm willing to tweak the QAPI naming rules within reason.

>> In terms of what we can enforce today, we can block ',',
>> but we can't block '.' without some cleanup, and possibly
>> the same for ':'. Can we assume we don't have any other
>> non-alphanumeric chars used ?

I ran qom-list-types (without 'abstract': true) for all 31
qemu-system-FOO, extracted the type names, and sorted them into buckets.

* I found 3255 distinct names

* 2445 names conform to the QAPI naming rule "only letters, digits,
  hyphen, and underscore, starting with a letter"

* 157 more names conform with the enum exception "may start with a
  digit"

* The remainder contain unwanted characters

  - 9 contain ',' and no other unwanted characters

    My "hw: Replace anti-social QOM type names (again)" fixes them.

  - 638 contain '.' and no other unwanted characters

    That's a lot.

    Perhaps we can permit '.' in enum names.  Needs thought.

  - 6 contain '.' and '+'

    Sun-UltraSparc-IIIi+-sparc64-cpu
    Sun-UltraSparc-IV+-sparc64-cpu
    power5+_v2.1-powerpc64-cpu
    power5+_v2.1-spapr-cpu-core
    power7+_v2.1-powerpc64-cpu
    power7+_v2.1-spapr-cpu-core

    Spell out "plus"?

I found no names with ':'.  Looks like we use ':' only for abstract
types (which includes interfaces).

The only #define TYPE_FOO with a colon I can see is

    #define TYPE_RAM_DISCARD_MANAGER "qemu:ram-discard-manager"

An interface type.  Let's ditch the "qemu:".

There are a bunch of interface names containing "::".  These come from
type_initialize_interface():

    info.name = g_strdup_printf("%s::%s", ti->name, interface_type->name);

>> If so, I think that today we we could probably get away with
>> saying:
>> 
>>    All QOM type names must begin with a letter, and contain
>>    only ASCII letters, digits, hyphen, underscore, period
>>    and colon. Usage of period and colon is deprecated.

I think we should reserve colon for QOM internal use.

[...]