Re: [Qemu-devel] [PATCH v12 27/36] qapi: Forbid case-insensitive clashes

[Eric Blake]

On 11/19/2015 09:50 AM, Markus Armbruster wrote:
> Let's think through this on a higher level.
> 
> I figure the motivation for this patch is twofold:
> 
> 1. C identifier clash detection
> 

> 
> 2. Dislike for interfaces that differ only in case

And the related dislike for interfaces that differ only in '-' or '_'.

> Our naming conventions actually make clashes due to folding relatively
> unlikely.  They are:
> 
> * All names consist of letters, digits, '-' and '_', starting with a
>   letter.
> 
> * Event names are ALL_CAPS, i.e. they use neither lower case nor '-'.
> 
>   Without lower case, '-' and '.', clash due to folding is impossible.

So if we enforce that convention, we don't have to worry about case
clash, and I don't think we have any outliers to whitelist.

> 
> * Command names, member names and built-in type names are dashed-lower,
>   i.e. they use neither upper case nor '_'.
> 
>   Without upper case, '_', and '.', clash due to folding is impossible.

If we enforce that convention, we have to whitelist outliers (for
example, 'netdev_add' as a command name, or a number of types whose
members still use '_').  The existence of a whitelist will discourage
future clashes, without any of the hassle of coding up clash checks.

> 
> * Type names are CamelCase.  They do not use '_' or '-'.
> 
>   Can theoretically clash in amusing ways: ONegus, OneGus.
>   Ain'tCamelCaseFun!

Do we have any outliers?

> 
>   We can get rid of the clashes by not case folding type names.  See
>   "[PATCH RFC 3/5] qapi: Use common name mangling for enumeration
>   constants".  The patch additionally drops folding of enumeration
>   member names, which isn't necessary.  Controversial.

There is even the possibility of mixed treatment (enumeration name
portion as-is, member name portion shouted, as in 'MyTypeVALUE1').  Not
sure if we'll get a clear answer to the controversy, but also not sure
it is worth holding up other patches while discussing that.

> 
> * Additionally, any name may be prefixed by __RFQDN_.  RFQDN consists of
>   letters, digits '-' and '.'.
> 
>   Because unprefixed names start with a letter, and the prefix starts
>   with '__', prefixed names cannot clash with unprefixed names.
> 
>   If we additionally stipulated that event prefixes are upper case, and
>   all others lower case, prefixes couldn't contribute to clashes at all.

It's a bit weird that we'd have __org.qemu_command and __ORG.QEMU_EVENT
for the same vendor.  On the other hand, FQDN is already
case-insensitive (qemu.org and QEMU.ORG resolve to the same address).
So there won't be clashes between vendors (as no two vendors can share a
FQDN that differs in case); beyond that, if a vendor wants to play weird
case games, that's their (downstream) problem, not ours.

> 
> Strict adherence to our naming conventions would eliminate clashes due
> to folding except for type names.  A single extra dictionary mapping
> c_name(typ.name).lower() to typ would suffice.

Certainly may be simpler than carrying three dictionaries for
command/event/type.

> 
> What happens to the rest of your queue if we shelve this patch for now?

Not much; in fact, according to the patch version log:

---
v12: add in entity case collisions (required sharing two maps),
improve commit message
v11: rebase to latest, don't focus so hard on future case-insensitive
extensions, adjust commit message
v10: new patch
---

I only even added it due to conversations on v9, and intentionally kept
it separate from 'qapi: Detect collisions in C member names' (we
absolutely want to report exact-name collisions, whether or not we also
decide to report case collisions).  It should not be a problem to defer
this patch (or a better version of it that enforces conventions, adds
whitelist handling, then only worries about type name case collisions)
to much later, if at all.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature