All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v12 27/36] qapi: Forbid case-insensitive clashes
Date: Thu, 19 Nov 2015 10:16:51 -0700	[thread overview]
Message-ID: <564E0403.5030602@redhat.com> (raw)
In-Reply-To: <8737w23pcr.fsf@blackfin.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 4072 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2015-11-19 17:17 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-18  8:52 [Qemu-devel] [PATCH v12 00/36] qapi member collision, alternate layout (post-introspection cleanups, subset D) Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 01/36] qapi: Track simple union tag in object.local_members Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 02/36] qapi-types: Consolidate gen_struct() and gen_union() Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 03/36] qapi-types: Simplify gen_struct_field[s] Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 04/36] qapi: Drop obsolete tag value collision assertions Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 05/36] qapi: Simplify QAPISchemaObjectTypeMember.check() Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 06/36] qapi: Clean up after previous commit Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 07/36] qapi: Fix up commit 7618b91's clash sanity checking change Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 08/36] qapi: Eliminate QAPISchemaObjectType.check() variable members Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 09/36] qapi: Factor out QAPISchemaObjectTypeMember.check_clash() Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 10/36] qapi: Simplify QAPISchemaObjectTypeVariants.check() Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 11/36] qapi: Check for qapi collisions involving variant members Eric Blake
2015-11-18 10:01   ` Markus Armbruster
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 12/36] qapi: Factor out QAPISchemaObjectType.check_clash() Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 13/36] qapi: Hoist tag collision check to Variants.check() Eric Blake
2015-11-18 10:08   ` Markus Armbruster
2015-11-18 10:24     ` Daniel P. Berrange
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 14/36] qapi: Remove outdated tests related to QMP/branch collisions Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 15/36] qapi: Track owner of each object member Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 16/36] qapi: Detect collisions in C member names Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 17/36] qapi: Fix c_name() munging Eric Blake
2015-11-18 10:18   ` Markus Armbruster
2015-11-18 16:20     ` Eric Blake
2015-11-18 17:59       ` Markus Armbruster
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 18/36] qapi: Remove dead visitor code Eric Blake
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 19/36] blkdebug: Merge hand-rolled and qapi BlkdebugEvent enum Eric Blake
2015-11-18 10:30   ` Markus Armbruster
2015-11-18 12:08     ` Kevin Wolf
2015-11-18 16:26       ` Eric Blake
2015-11-18 16:34         ` Kevin Wolf
2015-11-18 18:04           ` Markus Armbruster
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 20/36] blkdebug: Avoid '.' in enum values Eric Blake
2015-11-19  7:32   ` Markus Armbruster
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 21/36] qapi: Tighten the regex on valid names Eric Blake
2015-11-18 11:51   ` Markus Armbruster
2015-11-18 16:57     ` Eric Blake
2015-11-18 18:08       ` Markus Armbruster
2015-11-18 21:45   ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2015-11-19  8:10     ` Markus Armbruster
2015-11-19 23:25       ` Eric Blake
2015-11-20  6:16         ` Markus Armbruster
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 22/36] qapi: Don't let implicit enum MAX member collide Eric Blake
2015-11-18 10:54   ` Juan Quintela
2015-11-18 13:12   ` Markus Armbruster
2015-11-18 17:00     ` Eric Blake
2015-11-18 18:09       ` Markus Armbruster
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 23/36] qapi: Remove dead tests for max collision Eric Blake
2015-11-19  7:42   ` Markus Armbruster
2015-11-18  8:52 ` [Qemu-devel] [PATCH v12 24/36] cpu: Convert CpuInfo into flat union Eric Blake
2015-11-19 16:12   ` Markus Armbruster
2015-11-19 16:46     ` Eric Blake
2015-11-19 17:03       ` Markus Armbruster
2016-02-02 12:25   ` James Hogan
2016-02-02 13:49     ` Markus Armbruster
2016-02-02 14:49     ` Eric Blake
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 25/36] qapi: Add alias for ErrorClass Eric Blake
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 26/36] qapi: Change munging of CamelCase enum values Eric Blake
2015-11-18 13:46   ` Markus Armbruster
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 27/36] qapi: Forbid case-insensitive clashes Eric Blake
2015-11-18 17:08   ` Markus Armbruster
2015-11-18 22:09     ` Eric Blake
2015-11-18 22:22       ` Eric Blake
2015-11-18 22:52   ` Eric Blake
2015-11-18 22:55   ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2015-11-19 13:32     ` Markus Armbruster
2015-11-19 15:20       ` Eric Blake
2015-11-19 16:50   ` [Qemu-devel] [PATCH v12 27/36] " Markus Armbruster
2015-11-19 17:16     ` Eric Blake [this message]
2015-11-19 18:25       ` Markus Armbruster
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 28/36] qapi: Simplify QObject Eric Blake
2015-11-18 18:23   ` Markus Armbruster
2015-11-18 23:47   ` [Qemu-devel] [PATCH] fixup? " Eric Blake
2015-11-19  8:58     ` Markus Armbruster
2015-11-19  9:12       ` Markus Armbruster
2015-11-19 14:19       ` Eric Blake
2015-11-19 14:43         ` Markus Armbruster
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 29/36] qobject: Rename qtype_code to QType Eric Blake
2015-11-18 18:25   ` Markus Armbruster
2015-11-18 23:27     ` Eric Blake
2015-11-19  7:44       ` Markus Armbruster
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 30/36] qapi: Convert QType into qapi builtin enum type Eric Blake
2015-11-18 18:40   ` Markus Armbruster
2015-11-18 19:14   ` Markus Armbruster
2015-11-19  0:19   ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 31/36] qapi: Simplify visiting of alternate types Eric Blake
2015-11-18 18:46   ` Markus Armbruster
2015-11-18 20:08     ` Eric Blake
2015-11-19  8:01       ` Markus Armbruster
2015-11-19 14:08         ` Eric Blake
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 32/36] qapi: Inline _make_implicit_tag() Eric Blake
2015-11-18 18:48   ` Markus Armbruster
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 33/36] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 34/36] qapi: Add positive tests to qapi-schema-test Eric Blake
2015-11-20 22:49   ` Eric Blake
2015-11-23  7:34     ` Markus Armbruster
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 35/36] qapi: Simplify visits of optional fields Eric Blake
2015-11-18 18:54   ` Markus Armbruster
2015-11-18 20:10     ` Eric Blake
2015-11-18  8:53 ` [Qemu-devel] [PATCH v12 36/36] qapi: Shorter " Eric Blake
2015-11-18 19:04   ` Markus Armbruster
2015-11-18 19:19 ` [Qemu-devel] [PATCH v12 00/36] qapi member collision, alternate layout (post-introspection cleanups, subset D) Markus Armbruster
2015-11-19  0:25   ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=564E0403.5030602@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.