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