Re: [Qemu-devel] [PATCH 04/11] Add KVM support for S390x

[Markus Armbruster]

malc <address@hidden> writes:

> On Wed, 2 Dec 2009, Alexander Graf wrote:
>
>> 
>> On 02.12.2009, at 09:12, Aurelien Jarno wrote:
>> 
>> > On Mon, Nov 30, 2009 at 11:25:03PM +0100, Alexander Graf wrote:
>> >> 
>> >> On 30.11.2009, at 19:18, Aurelien Jarno wrote:
>> >> 
>> >>> On Thu, Nov 26, 2009 at 02:23:13PM +0100, Alexander Graf wrote:
[...]
>> >>>> +
>> >>>> +static void _kvm_s390_interrupt(CPUState *env, int type, uint32_t 
>> >>>> parm, uint64_t parm64, int vm)
>> >>>> +{
>> >>> 
>> >>> Why such a name starting with an underscore?
>> >> 
>> >> Because that's the internal function that gets used by the exported, 
>> >> properly named ones. Are there any conventions on how to declare private 
>> >> functions?
>> > 
>> > I don't think there is any convention, but I know malc always complains
>> > about not introducing names starting with an underscore.
>
> Yeah he does.
>
>> 
>> Hm - I just wanted to clearly show that this is an internal API, nobody 
>> should really have to call directly. But I'm open for other naming 
>> suggestions.
>
> Thing is, in 7.1.3#1 standard says (after explicitly reserving __ _[A-Z]
> for any use):
>          -- All  identifiers  that  begin  with  an  underscore are
>             always reserved for use as identifiers with file  scope
>             in both the ordinary and tag name spaces.
>
> And i could never really understand (or recall/comprehend when asked
> and being given an answer) what this entails. (Anyone?)

Later in 7.1.3:

    If the program declares or defines an identifier in a context in
    which it is reserved (other than as allowed by 7.1.4), or defines a
    reserved identifier as a macro name, the behavior is undefined.

This gives implementations of the standard (compiler + libc) license to
use reserved identifiers for their own purposes.  If they clash with the
user's identifiers, and things break, the user gets to keep the pieces.

Now, it's quite unlikely that _kvm_s390_interrupt() clashes with
anything in practice.  It does, however, set a bad example.

> So i would go with something imaginative like internal_do_not_use_kvm*,
> but that's just me. You can go wild here, leading underscore doesn't look
> attractive though.

Why not kvm_s390_interrupt_internal(), or even kvm_s390_interrupt_()?