[Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

We copied HACKING from libvirt but it has some bogus stuff:
neither underscore capital, double underscore, or underscore 't' suffixes
are reserved in Posix/C: this appears to be based on misreading of the
C standard. Using sane prefixes is enough to avoid conflicts.

These rules are also widely violated in our codebase,
and it does not make sense to rework it all, apparently for
no benefit.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 HACKING | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/HACKING b/HACKING
index 471cf1d..0a941fc 100644
--- a/HACKING
+++ b/HACKING
@@ -69,10 +69,6 @@ it points to, or it is aliased to another pointer that is.
 2.3. Typedefs
 Typedefs are used to eliminate the redundant 'struct' keyword.
 
-2.4. Reserved namespaces in C and POSIX
-Underscore capital, double underscore, and underscore 't' suffixes should be
-avoided.
-
 3. Low level memory management
 
 Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
-- 
MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

On Tue, Aug 28, 2012 at 07:01:16PM +0300, Michael S. Tsirkin wrote:
> We copied HACKING from libvirt but it has some bogus stuff:
> neither underscore capital, double underscore, or underscore 't' suffixes
> are reserved in Posix/C: this appears to be based on misreading of the
> C standard. Using sane prefixes is enough to avoid conflicts.
> 
> These rules are also widely violated in our codebase,

To add to that, they are even contradicted in HACKING itself
which suggests using ram_addr_t for RAM offsets (_t suffix).

> and it does not make sense to rework it all, apparently for
> no benefit.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  HACKING | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/HACKING b/HACKING
> index 471cf1d..0a941fc 100644
> --- a/HACKING
> +++ b/HACKING
> @@ -69,10 +69,6 @@ it points to, or it is aliased to another pointer that is.
>  2.3. Typedefs
>  Typedefs are used to eliminate the redundant 'struct' keyword.
>  
> -2.4. Reserved namespaces in C and POSIX
> -Underscore capital, double underscore, and underscore 't' suffixes should be
> -avoided.
> -
>  3. Low level memory management
>  
>  Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
> -- 
> MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Peter Maydell]

On 28 August 2012 17:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> We copied HACKING from libvirt but it has some bogus stuff:
> neither underscore capital, double underscore, or underscore 't' suffixes
> are reserved in Posix/C: this appears to be based on misreading of the
> C standard. Using sane prefixes is enough to avoid conflicts.

> -2.4. Reserved namespaces in C and POSIX
> -Underscore capital, double underscore, and underscore 't' suffixes should be
> -avoided.

I think this is just a missing "prefixes". C99 7.1.3
reserves underscore capital and double underscore prefixes.
POSIX reserves _t if any POSIX header is defined (POSIX.1-2008 section
2.2.2, http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html)

I would suggest that the section is reworded to:

# Underscore capital and double underscore prefixes are reserved
# by the C standard. Underscore 't' suffixes are reserved by POSIX.
# They should be avoided in QEMU code.

-- PMM

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

On Tue, Aug 28, 2012 at 05:24:40PM +0100, Peter Maydell wrote:
> On 28 August 2012 17:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> > We copied HACKING from libvirt but it has some bogus stuff:
> > neither underscore capital, double underscore, or underscore 't' suffixes
> > are reserved in Posix/C: this appears to be based on misreading of the
> > C standard. Using sane prefixes is enough to avoid conflicts.

> > -2.4. Reserved namespaces in C and POSIX
> > -Underscore capital, double underscore, and underscore 't' suffixes should be
> > -avoided.
> 
> I think this is just a missing "prefixes". C99 7.1.3
> reserves underscore capital and double underscore prefixes.

This is taking it out of context - reserved means different
things in different parts of the spec.

As far as I can see, 7.1.3 talks about what is permissible in headers:
naturally when a libc implementation adds stuff in a header it would
want to avoid breaking applications including this header. But
it does not talk about what is permissible in a legal program - to avoid
conflicts, you just need to use variables with reasonable names like
kvmXXX qemuXXX or virtioXXX.

So it does not look like this is a reason to not use __kvm_pv_eoi_disabled.

Chances that a libc header predefines this name are zero.

> POSIX reserves _t if any POSIX header is defined (POSIX.1-2008 section
> 2.2.2, http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html)

Same thing really.
So this says posix implementations can add types ending with _t in any
header.  It does not prohibit applications from using such names
in a reasonable manner, and it does not look like this is a reason to
not use qemu_foo_bar_t.


> I would suggest that the section is reworded to:
> 
> # Underscore capital and double underscore prefixes are reserved
> # by the C standard. Underscore 't' suffixes are reserved by POSIX.
> # They should be avoided in QEMU code.
> 
> -- PMM

This might be nice from language purism point of view
but ignores the fact that QEMU already uses lots of these.
Specifically a _t type is even mentioned in HACKING itself.

So the benefit to excluding this in new code is marginal.


-- 
MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Peter Maydell]

On 28 August 2012 18:18, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Aug 28, 2012 at 05:24:40PM +0100, Peter Maydell wrote:
>>  C99 7.1.3
>> reserves underscore capital and double underscore prefixes.
>
> This is taking it out of context - reserved means different
> things in different parts of the spec.
>
> As far as I can see, 7.1.3 talks about what is permissible in headers:
> naturally when a libc implementation adds stuff in a header it would
> want to avoid breaking applications including this header. But
> it does not talk about what is permissible in a legal program - to avoid
> conflicts, you just need to use variables with reasonable names like
> kvmXXX qemuXXX or virtioXXX.

7.1.3 para 2 says "If the program declares or defines an identifier
in a context in which it is reserved [...] the behavior is undefined."
The rationale for 7.1.3 says that the underscore-cap and double
underscore identifiers are "reserved for the implementor" and that
"part of the name space of internal identifiers beginning with
underscore is available to the user" which implies that the rest
of that namespace is *not* available to the user.

I think that's fairly clear that we can't use these identifiers
without entering the realm of undefined behavior.

-- PMM

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

On Tue, Aug 28, 2012 at 06:23:38PM +0100, Peter Maydell wrote:
> On 28 August 2012 18:18, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Aug 28, 2012 at 05:24:40PM +0100, Peter Maydell wrote:
> >>  C99 7.1.3
> >> reserves underscore capital and double underscore prefixes.
> >
> > This is taking it out of context - reserved means different
> > things in different parts of the spec.
> >
> > As far as I can see, 7.1.3 talks about what is permissible in headers:
> > naturally when a libc implementation adds stuff in a header it would
> > want to avoid breaking applications including this header. But
> > it does not talk about what is permissible in a legal program - to avoid
> > conflicts, you just need to use variables with reasonable names like
> > kvmXXX qemuXXX or virtioXXX.
> 
> 7.1.3 para 2 says "If the program declares or defines an identifier
> in a context in which it is reserved [...] the behavior is undefined."
> The rationale for 7.1.3 says that the underscore-cap and double
> underscore identifiers are "reserved for the implementor" and that
> "part of the name space of internal identifiers beginning with
> underscore is available to the user" which implies that the rest
> of that namespace is *not* available to the user.
> 
> I think that's fairly clear that we can't use these identifiers
> without entering the realm of undefined behavior.
> 

I'll have to re-read that, I missed it.
What about _t in POSIX? That seems fairly safe if name is long and qemu
specific enough.

-- 
MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Peter Maydell]

On 28 August 2012 18:32, Michael S. Tsirkin <mst@redhat.com> wrote:
> What about _t in POSIX? That seems fairly safe if name is long and qemu
> specific enough.

Depends what you mean by "safe". The spec says "don't use this";
it isn't any different to the __ and _[A-Z] prohibitions in that
respect.

Other posix namespace landgrabs you may not have expected:
 * ctype.h reserves "is[a-z]" and to[a-z]" prefixes
 * string.h takes "str[a-z]" and "mem[a-z]" prefixes

(and qemu-common.h includes both string.h and ctype.h so this
effectively applies to all of qemu).

I'm in two minds about these, because on the one hand they're
reserved but on the other hand abiding by the rules makes things
uglier (whereas "use one underscore not two" doesn't produce
an uglier name IMHO).

(posix also if I'm reading it correctly reserves all of 'prefix
underscore', which is annoying.)

-- PMM

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

On Tue, Aug 28, 2012 at 06:46:53PM +0100, Peter Maydell wrote:
> On 28 August 2012 18:32, Michael S. Tsirkin <mst@redhat.com> wrote:
> > What about _t in POSIX? That seems fairly safe if name is long and qemu
> > specific enough.
> 
> Depends what you mean by "safe". The spec says "don't use this";
> it isn't any different to the __ and _[A-Z] prohibitions in that
> respect.

It's different: unlike C compiler POSIX can not mangle
names in an application. So it will not internally
create qemu_foo_bar_t from qemu_foo_bar, and thus
qemu_foo_bar_t is very unlikely to create conflicts.

OTOH compiler people do mysterious and strange things
to the point where you do not want to mess with them
if you can.

> Other posix namespace landgrabs you may not have expected:
>  * ctype.h reserves "is[a-z]" and to[a-z]" prefixes
>  * string.h takes "str[a-z]" and "mem[a-z]" prefixes
> 
> (and qemu-common.h includes both string.h and ctype.h so this
> effectively applies to all of qemu).
> 
> I'm in two minds about these, because on the one hand they're
> reserved but on the other hand abiding by the rules makes things
> uglier (whereas "use one underscore not two" doesn't produce
> an uglier name IMHO).
> 
> (posix also if I'm reading it correctly reserves all of 'prefix
> underscore', which is annoying.)
> 
> -- PMM

Exactly my points.

-- 
MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[malc]

On Tue, 28 Aug 2012, Michael S. Tsirkin wrote:

> On Tue, Aug 28, 2012 at 05:24:40PM +0100, Peter Maydell wrote:
> > On 28 August 2012 17:01, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > We copied HACKING from libvirt but it has some bogus stuff:
> > > neither underscore capital, double underscore, or underscore 't' suffixes
> > > are reserved in Posix/C: this appears to be based on misreading of the
> > > C standard. Using sane prefixes is enough to avoid conflicts.
> 
> > > -2.4. Reserved namespaces in C and POSIX
> > > -Underscore capital, double underscore, and underscore 't' suffixes should be
> > > -avoided.
> > 
> > I think this is just a missing "prefixes". C99 7.1.3
> > reserves underscore capital and double underscore prefixes.
> 
> This is taking it out of context - reserved means different
> things in different parts of the spec.

Reserved means - reserved for implementation to use as it sees fit,
for instance - 6.2.5 

28) Implementation-defined keywords shall have the form of an identifier
    reserved for any use as described in 7.1.3.

Future versions of C might define something in the form __xxx or _[A-Z]xxx 
precisely because those are invalid for user to define.. (_Imaginary, 
_Bool, whatever)

7.1.3#2

> 
> As far as I can see, 7.1.3 talks about what is permissible in headers: 

And you are wrong.

> naturally when a libc implementation adds stuff in a header it would 
> want to avoid breaking applications including this header. But it does 
> not talk about what is permissible in a legal program - to avoid 
> conflicts, you just need to use variables with reasonable names like 
> kvmXXX qemuXXX or virtioXXX.
> 
> So it does not look like this is a reason to not use __kvm_pv_eoi_disabled.
> 
> Chances that a libc header predefines this name are zero.
> 
> > POSIX reserves _t if any POSIX header is defined (POSIX.1-2008 section
> > 2.2.2, http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html)
> 
> Same thing really.
> So this says posix implementations can add types ending with _t in any
> header.  It does not prohibit applications from using such names
> in a reasonable manner, and it does not look like this is a reason to
> not use qemu_foo_bar_t.
> 
> 
> > I would suggest that the section is reworded to:
> > 
> > # Underscore capital and double underscore prefixes are reserved
> > # by the C standard. Underscore 't' suffixes are reserved by POSIX.
> > # They should be avoided in QEMU code.
> > 
> > -- PMM
> 
> This might be nice from language purism point of view
> but ignores the fact that QEMU already uses lots of these.
> Specifically a _t type is even mentioned in HACKING itself.
> 
> So the benefit to excluding this in new code is marginal.
> 
> 
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Blue Swirl]

On Tue, Aug 28, 2012 at 4:01 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> We copied HACKING from libvirt but it has some bogus stuff:
> neither underscore capital, double underscore, or underscore 't' suffixes
> are reserved in Posix/C: this appears to be based on misreading of the
> C standard. Using sane prefixes is enough to avoid conflicts.
>
> These rules are also widely violated in our codebase,
> and it does not make sense to rework it all, apparently for
> no benefit.

NACK. The benefit is improved standards compliance. One day we could
find QEMU being ported to environment which conflicts with these
symbols.

The tiny single benefit from violating the rules would be that you
could use a few additional possible classes of prefixes, in addition
to the infinite combinations already available.

>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  HACKING | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/HACKING b/HACKING
> index 471cf1d..0a941fc 100644
> --- a/HACKING
> +++ b/HACKING
> @@ -69,10 +69,6 @@ it points to, or it is aliased to another pointer that is.
>  2.3. Typedefs
>  Typedefs are used to eliminate the redundant 'struct' keyword.
>
> -2.4. Reserved namespaces in C and POSIX
> -Underscore capital, double underscore, and underscore 't' suffixes should be
> -avoided.
> -
>  3. Low level memory management
>
>  Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
> --
> MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

On Tue, Aug 28, 2012 at 05:13:24PM +0000, Blue Swirl wrote:
> On Tue, Aug 28, 2012 at 4:01 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > We copied HACKING from libvirt but it has some bogus stuff:
> > neither underscore capital, double underscore, or underscore 't' suffixes
> > are reserved in Posix/C: this appears to be based on misreading of the
> > C standard. Using sane prefixes is enough to avoid conflicts.
> >
> > These rules are also widely violated in our codebase,
> > and it does not make sense to rework it all, apparently for
> > no benefit.
> 
> NACK. The benefit is improved standards compliance. One day we could
> find QEMU being ported to environment which conflicts with these
> symbols.

We are talking about stuff like __kvm_pv_eoi - so the chance is exactly 0.
And if it does happen then you run a simple script and fix
this one instance.

> The tiny single benefit from violating the rules would be that you
> could use a few additional possible classes of prefixes, in addition
> to the infinite combinations already available.

Benefit would be consistency with existing QEMU code
which has both _t __  and _X, and consistency
within HACKING itself.

> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  HACKING | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/HACKING b/HACKING
> > index 471cf1d..0a941fc 100644
> > --- a/HACKING
> > +++ b/HACKING
> > @@ -69,10 +69,6 @@ it points to, or it is aliased to another pointer that is.
> >  2.3. Typedefs
> >  Typedefs are used to eliminate the redundant 'struct' keyword.
> >
> > -2.4. Reserved namespaces in C and POSIX
> > -Underscore capital, double underscore, and underscore 't' suffixes should be
> > -avoided.
> > -
> >  3. Low level memory management
> >
> >  Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
> > --
> > MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Blue Swirl]

On Tue, Aug 28, 2012 at 5:21 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Aug 28, 2012 at 05:13:24PM +0000, Blue Swirl wrote:
>> On Tue, Aug 28, 2012 at 4:01 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > We copied HACKING from libvirt but it has some bogus stuff:
>> > neither underscore capital, double underscore, or underscore 't' suffixes
>> > are reserved in Posix/C: this appears to be based on misreading of the
>> > C standard. Using sane prefixes is enough to avoid conflicts.
>> >
>> > These rules are also widely violated in our codebase,
>> > and it does not make sense to rework it all, apparently for
>> > no benefit.
>>
>> NACK. The benefit is improved standards compliance. One day we could
>> find QEMU being ported to environment which conflicts with these
>> symbols.
>
> We are talking about stuff like __kvm_pv_eoi - so the chance is exactly 0.

How do you know for sure? The implementation could be that symbols are
converted by the compiler to use underscore prefixes.

> And if it does happen then you run a simple script and fix
> this one instance.

Then why not fix now? How about kvm_pv_eoi_state?

>
>> The tiny single benefit from violating the rules would be that you
>> could use a few additional possible classes of prefixes, in addition
>> to the infinite combinations already available.
>
> Benefit would be consistency with existing QEMU code
> which has both _t __  and _X, and consistency
> within HACKING itself.

There are plenty of bad things wrt style, but the plan is to get rid
of those eventually. It takes time since global reformatting etc. to
enforce the rules is not accepted.

>
>> >
>> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> > ---
>> >  HACKING | 4 ----
>> >  1 file changed, 4 deletions(-)
>> >
>> > diff --git a/HACKING b/HACKING
>> > index 471cf1d..0a941fc 100644
>> > --- a/HACKING
>> > +++ b/HACKING
>> > @@ -69,10 +69,6 @@ it points to, or it is aliased to another pointer that is.
>> >  2.3. Typedefs
>> >  Typedefs are used to eliminate the redundant 'struct' keyword.
>> >
>> > -2.4. Reserved namespaces in C and POSIX
>> > -Underscore capital, double underscore, and underscore 't' suffixes should be
>> > -avoided.
>> > -
>> >  3. Low level memory management
>> >
>> >  Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
>> > --
>> > MST

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Peter Maydell]

On 28 August 2012 18:21, Michael S. Tsirkin <mst@redhat.com> wrote:
> We are talking about stuff like __kvm_pv_eoi - so the chance is exactly 0.
> And if it does happen then you run a simple script and fix
> this one instance.

Why not just use a name that doesn't use a double underscore
in the first place? The C standard specifically allows single
underscore + lowercase to give things other than the implementation
part of the underscore-namespace. In this case, "_kvm_pv_eoi"
would be OK.

>> The tiny single benefit from violating the rules would be that you
>> could use a few additional possible classes of prefixes, in addition
>> to the infinite combinations already available.
>
> Benefit would be consistency with existing QEMU code
> which has both _t __  and _X, and consistency
> within HACKING itself.

HACKING and CODING_STYLE contain a number of rules which
the existing codebase doesn't fully conform to. The idea
is to incrementally improve consistency and correctness.

-- PMM

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[malc]

On Tue, 28 Aug 2012, Peter Maydell wrote:

> On 28 August 2012 18:21, Michael S. Tsirkin <mst@redhat.com> wrote:
> > We are talking about stuff like __kvm_pv_eoi - so the chance is exactly 0.
> > And if it does happen then you run a simple script and fix
> > this one instance.
> 
> Why not just use a name that doesn't use a double underscore
> in the first place? The C standard specifically allows single
> underscore + lowercase to give things other than the implementation
> part of the underscore-namespace. In this case, "_kvm_pv_eoi"
> would be OK.

No it wouldn't, _kvm_pv_eoi is a file scope identifier, and names 
beginning with underscore are reserved in this context.

> 
> >> The tiny single benefit from violating the rules would be that you
> >> could use a few additional possible classes of prefixes, in addition
> >> to the infinite combinations already available.
> >
> > Benefit would be consistency with existing QEMU code
> > which has both _t __  and _X, and consistency
> > within HACKING itself.
> 
> HACKING and CODING_STYLE contain a number of rules which
> the existing codebase doesn't fully conform to. The idea
> is to incrementally improve consistency and correctness.
> 
> -- PMM
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

On Tue, Aug 28, 2012 at 09:33:16PM +0400, malc wrote:
> On Tue, 28 Aug 2012, Peter Maydell wrote:
> 
> > On 28 August 2012 18:21, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > We are talking about stuff like __kvm_pv_eoi - so the chance is exactly 0.
> > > And if it does happen then you run a simple script and fix
> > > this one instance.
> > 
> > Why not just use a name that doesn't use a double underscore
> > in the first place? The C standard specifically allows single
> > underscore + lowercase to give things other than the implementation
> > part of the underscore-namespace. In this case, "_kvm_pv_eoi"
> > would be OK.
> 
> No it wouldn't, _kvm_pv_eoi is a file scope identifier, and names 
> beginning with underscore are reserved in this context.

Looks like they are and I missed that.
Maybe we should add that to HACKING.

> > 
> > >> The tiny single benefit from violating the rules would be that you
> > >> could use a few additional possible classes of prefixes, in addition
> > >> to the infinite combinations already available.
> > >
> > > Benefit would be consistency with existing QEMU code
> > > which has both _t __  and _X, and consistency
> > > within HACKING itself.
> > 
> > HACKING and CODING_STYLE contain a number of rules which
> > the existing codebase doesn't fully conform to. The idea
> > is to incrementally improve consistency and correctness.
> > 
> > -- PMM
> > 
> 
> -- 
> mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Michael S. Tsirkin]

On Tue, Aug 28, 2012 at 06:27:59PM +0100, Peter Maydell wrote:
> On 28 August 2012 18:21, Michael S. Tsirkin <mst@redhat.com> wrote:
> > We are talking about stuff like __kvm_pv_eoi - so the chance is exactly 0.
> > And if it does happen then you run a simple script and fix
> > this one instance.
> 
> Why not just use a name that doesn't use a double underscore
> in the first place? The C standard specifically allows single
> underscore + lowercase to give things other than the implementation
> part of the underscore-namespace. In this case, "_kvm_pv_eoi"
> would be OK.

BTW this is exactly what v2 of my patch did but Blue Swirl nacked this too.

> >> The tiny single benefit from violating the rules would be that you
> >> could use a few additional possible classes of prefixes, in addition
> >> to the infinite combinations already available.
> >
> > Benefit would be consistency with existing QEMU code
> > which has both _t __  and _X, and consistency
> > within HACKING itself.
> 
> HACKING and CODING_STYLE contain a number of rules which
> the existing codebase doesn't fully conform to. The idea
> is to incrementally improve consistency and correctness.
> 
> -- PMM

How about fixing HACKING itself? It recommends using ram_addr_t.

Re: [Qemu-devel] [PATCH] HACKING: remove bogus restrictions

[Andreas Färber]

Am 28.08.2012 18:01, schrieb Michael S. Tsirkin:
> We copied HACKING from libvirt but it has some bogus stuff:
> neither underscore capital, double underscore, or underscore 't' suffixes
> are reserved in Posix/C: this appears to be based on misreading of the
> C standard. Using sane prefixes is enough to avoid conflicts.
> 
> These rules are also widely violated in our codebase,
> and it does not make sense to rework it all, apparently for
> no benefit.

It was generally not accepted to do Coding Style cleanups in the code
base so you will find lots of violations for any rule if you invest the
time to search. That's no reason to abolish any such rule.

Cc'ing malc, who has "violently" fought for this particular rule.

I also think that asking people not to introduce new ..._t typedefs is
different from asking to use appropriate existing typedefs, so HACKING
is not really contradicting itself in that aspect.

Regards,
Andreas

> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  HACKING | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/HACKING b/HACKING
> index 471cf1d..0a941fc 100644
> --- a/HACKING
> +++ b/HACKING
> @@ -69,10 +69,6 @@ it points to, or it is aliased to another pointer that is.
>  2.3. Typedefs
>  Typedefs are used to eliminate the redundant 'struct' keyword.
>  
> -2.4. Reserved namespaces in C and POSIX
> -Underscore capital, double underscore, and underscore 't' suffixes should be
> -avoided.
> -
>  3. Low level memory management
>  
>  Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg