Re: [Xen-devel] [PATCH 5/7] public / x86: introduce __HYPERCALL_iommu_op

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 06/07/2018 02:21 PM, Jan Beulich wrote:
>>>> On 07.06.18 at 13:42, <Paul.Durrant@xxxxxxxxxx> wrote:
>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>> Sent: 16 March 2018 12:25
>>>>>> On 12.02.18 at 11:47, <paul.durrant@xxxxxxxxxx> wrote:
>>>> --- a/xen/arch/x86/Makefile
>>>> +++ b/xen/arch/x86/Makefile
>>>> @@ -33,6 +33,7 @@ obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
>>>>  obj-y += hypercall.o
>>>>  obj-y += i387.o
>>>>  obj-y += i8259.o
>>>> +obj-y += iommu_op.o
>>>
>>> As mentioned in other contexts, I'd prefer if we stopped using
>>> underscores in places where dashes (or other separators not
>>> usable in C identifiers) are fine.
>>
>> I don't see any guidance in CODING_STYLE or elsewhere, and also the majority 
>> of the codebase seems to prefer using underscores in module names. 
>> Personally 
>> I'd prefer new code remain consistent.
> 
> The lack of statement to this effect is why I've said "I'd prefer". See
> alternative-asm.h, x86-defns.h, or x86-vendors.h for _recent_
> examples of moving into the other direction. On all keyboards I've
> seen or used, an underscore requires two keys to be pressed, while
> a dash takes only one. This isn't much for an individual instance, but
> it sums up. It's the same reason why I'm advocating against the use
> of underscores in new command line option names.
> 
> In the end, looking at the history of typography, I think underscore
> is a relatively late (and presumably artificial) addition; in particular I
> don't recall mechanical type writers to even have a key for it.

<pedantic>The mechanical typewriters I learned on had an underscore to
allow you to go back and underline words.</pedantic>

> It's
> use as a visual separator is necessary in e.g. programming
> languages, as commonly dash designated the operator for "minus"
> there. Extending such naming to non-identifiers (file system names
> and command line options are just prominent examples) is simply
> misguided imo.

Well in any case, maybe this should be discussed in a patch to
CODING_STYLE, rather than in the middle of a patch series about
something completely different.

>>>> +#ifndef __XEN_PUBLIC_IOMMU_OP_H__
>>>> +#define __XEN_PUBLIC_IOMMU_OP_H__
>>>
>>> Please can you avoid introducing further name space violations
>>> into the public headers?
>>
>> I assume you mean the leading '__'? Again, I chose the name based on 
>> consistency with other code and I'd prefer to remain consistent. Could you 
>> explain why having a leading '__' is problematic?
> 
> Names starting with double underscores are reserved (as are, btw,
> names starting with a single underscore and an upper case letter).
> While it's unlikely for a compiler to ever want to use
> __XEN_PUBLIC_IOMMU_OP_H__ for its internal purposes, we couldn't
> validly complain if one did.

I'm with Jan on this one.  At the moment I'm not sure about using dashes
instead of underscores for filenames, but in this case the extra
underscores at the beginning and end are redundant; the "XEN_..._H" is
sufficient to make the contents unique.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel