[Qemu-devel] [PATCH 3/5] CODING_STYLE: add memory management rules

[Qemu-devel] [PATCH 3/5] CODING_STYLE: add memory management rules

[Blue Swirl]

Add memory management rules, somewhat like libvirt HACKING.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 CODING_STYLE |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index 3f10d72..085c86f 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -148,3 +148,11 @@ up-front that this is a read-only pointer.  Perhaps more
 importantly, if we're diligent about this, when you see a non-const
 pointer, you're guaranteed that it is used to modify the storage
 it points to, or it is aliased to another pointer that is.
+
+
+7. Low level memory management
+
+Use of the malloc/free/realloc/calloc APIs is not allowed in the QEMU
+codebase. Instead of these routines, use the replacement
+qemu_malloc/qemu_mallocz/qemu_realloc/qemu_free or
+qemu_vmalloc/qemu_memalign/qemu_vfree APIs.
-- 
1.6.2.4

[Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[Blue Swirl]

On Thu, Aug 12, 2010 at 5:50 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> Add memory management rules, somewhat like libvirt HACKING.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  CODING_STYLE |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 3f10d72..085c86f 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -148,3 +148,11 @@ up-front that this is a read-only pointer.  Perhaps more
>  importantly, if we're diligent about this, when you see a non-const
>  pointer, you're guaranteed that it is used to modify the storage
>  it points to, or it is aliased to another pointer that is.
> +
> +
> +7. Low level memory management
> +
> +Use of the malloc/free/realloc/calloc APIs is not allowed in the QEMU

I forgot to add valloc/memalign/posix_memalign.

> +codebase. Instead of these routines, use the replacement
> +qemu_malloc/qemu_mallocz/qemu_realloc/qemu_free or
> +qemu_vmalloc/qemu_memalign/qemu_vfree APIs.

This should also mention that memory allocated by qemu_vmalloc or
qemu_memalign should be freed with qemu_vfree.

In general, whole of 7 is an existing rule and current codebase seems
to follow it.

This should be added as a strict rule (as opposed to guideline), since
breaking this will cause problems on Win32 and user emulators.

Re: [Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1806 bytes --]

On Fri, 13 Aug 2010, Blue Swirl wrote:

> On Thu, Aug 12, 2010 at 5:50 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> > Add memory management rules, somewhat like libvirt HACKING.
> >
> > Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> > ---
> >  CODING_STYLE |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index 3f10d72..085c86f 100644
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -148,3 +148,11 @@ up-front that this is a read-only pointer.  Perhaps more
> >  importantly, if we're diligent about this, when you see a non-const
> >  pointer, you're guaranteed that it is used to modify the storage
> >  it points to, or it is aliased to another pointer that is.
> > +
> > +
> > +7. Low level memory management
> > +
> > +Use of the malloc/free/realloc/calloc APIs is not allowed in the QEMU
> 
> I forgot to add valloc/memalign/posix_memalign.
> 
> > +codebase. Instead of these routines, use the replacement
> > +qemu_malloc/qemu_mallocz/qemu_realloc/qemu_free or
> > +qemu_vmalloc/qemu_memalign/qemu_vfree APIs.
> 
> This should also mention that memory allocated by qemu_vmalloc or
> qemu_memalign should be freed with qemu_vfree.
> 
> In general, whole of 7 is an existing rule and current codebase seems
> to follow it.
> 
> This should be added as a strict rule (as opposed to guideline), since
> breaking this will cause problems on Win32 and user emulators.
> 

This is all perfectly reasonable, but begs one question, where do we stop
codifying self-evident truths (or who is the ultimiate judge of what is
self-evident and what isn't), since taking the above to extreme we will
end up adding stuff like: you must use close to dispose of open-ed
descriptors and such like.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[Blue Swirl]

On Fri, Aug 13, 2010 at 9:01 PM, malc <av1474@comtv.ru> wrote:
> On Fri, 13 Aug 2010, Blue Swirl wrote:
>
>> On Thu, Aug 12, 2010 at 5:50 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>> > Add memory management rules, somewhat like libvirt HACKING.
>> >
>> > Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> > ---
>> >  CODING_STYLE |    8 ++++++++
>> >  1 files changed, 8 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/CODING_STYLE b/CODING_STYLE
>> > index 3f10d72..085c86f 100644
>> > --- a/CODING_STYLE
>> > +++ b/CODING_STYLE
>> > @@ -148,3 +148,11 @@ up-front that this is a read-only pointer.  Perhaps more
>> >  importantly, if we're diligent about this, when you see a non-const
>> >  pointer, you're guaranteed that it is used to modify the storage
>> >  it points to, or it is aliased to another pointer that is.
>> > +
>> > +
>> > +7. Low level memory management
>> > +
>> > +Use of the malloc/free/realloc/calloc APIs is not allowed in the QEMU
>>
>> I forgot to add valloc/memalign/posix_memalign.
>>
>> > +codebase. Instead of these routines, use the replacement
>> > +qemu_malloc/qemu_mallocz/qemu_realloc/qemu_free or
>> > +qemu_vmalloc/qemu_memalign/qemu_vfree APIs.
>>
>> This should also mention that memory allocated by qemu_vmalloc or
>> qemu_memalign should be freed with qemu_vfree.
>>
>> In general, whole of 7 is an existing rule and current codebase seems
>> to follow it.
>>
>> This should be added as a strict rule (as opposed to guideline), since
>> breaking this will cause problems on Win32 and user emulators.
>>
>
> This is all perfectly reasonable, but begs one question, where do we stop
> codifying self-evident truths (or who is the ultimiate judge of what is
> self-evident and what isn't), since taking the above to extreme we will
> end up adding stuff like: you must use close to dispose of open-ed
> descriptors and such like.

Interesting question. We could assume that the target audience knows
well how to write portable C, conformal to various standards, so we
only have to point out QEMU specific pitfalls. This approach would
probably mean that there would be no need to mention, for example, the
issues with identifiers starting with an underscore.

Another approach is to consider the history. Which types of problems
there have been frequently with the proposed patches? Then the
underscore issue (just as an example) should definitely be raised,
even though that is just one aspect of standards conformance.

I don't think either approach would solve your question though.

Re: [Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2909 bytes --]

On Fri, 13 Aug 2010, Blue Swirl wrote:

> On Fri, Aug 13, 2010 at 9:01 PM, malc <av1474@comtv.ru> wrote:
> > On Fri, 13 Aug 2010, Blue Swirl wrote:
> >
> >> On Thu, Aug 12, 2010 at 5:50 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> >> > Add memory management rules, somewhat like libvirt HACKING.
> >> >
> >> > Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> >> > ---
> >> >  CODING_STYLE |    8 ++++++++
> >> >  1 files changed, 8 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/CODING_STYLE b/CODING_STYLE
> >> > index 3f10d72..085c86f 100644
> >> > --- a/CODING_STYLE
> >> > +++ b/CODING_STYLE
> >> > @@ -148,3 +148,11 @@ up-front that this is a read-only pointer.  Perhaps more
> >> >  importantly, if we're diligent about this, when you see a non-const
> >> >  pointer, you're guaranteed that it is used to modify the storage
> >> >  it points to, or it is aliased to another pointer that is.
> >> > +
> >> > +
> >> > +7. Low level memory management
> >> > +
> >> > +Use of the malloc/free/realloc/calloc APIs is not allowed in the QEMU
> >>
> >> I forgot to add valloc/memalign/posix_memalign.
> >>
> >> > +codebase. Instead of these routines, use the replacement
> >> > +qemu_malloc/qemu_mallocz/qemu_realloc/qemu_free or
> >> > +qemu_vmalloc/qemu_memalign/qemu_vfree APIs.
> >>
> >> This should also mention that memory allocated by qemu_vmalloc or
> >> qemu_memalign should be freed with qemu_vfree.
> >>
> >> In general, whole of 7 is an existing rule and current codebase seems
> >> to follow it.
> >>
> >> This should be added as a strict rule (as opposed to guideline), since
> >> breaking this will cause problems on Win32 and user emulators.
> >>
> >
> > This is all perfectly reasonable, but begs one question, where do we stop
> > codifying self-evident truths (or who is the ultimiate judge of what is
> > self-evident and what isn't), since taking the above to extreme we will
> > end up adding stuff like: you must use close to dispose of open-ed
> > descriptors and such like.
> 
> Interesting question. We could assume that the target audience knows
> well how to write portable C, conformal to various standards, so we
> only have to point out QEMU specific pitfalls. This approach would
> probably mean that there would be no need to mention, for example, the
> issues with identifiers starting with an underscore.
>
> Another approach is to consider the history. Which types of problems
> there have been frequently with the proposed patches? Then the
> underscore issue (just as an example) should definitely be raised,
> even though that is just one aspect of standards conformance.

I.e. having a (N)FAQ(BNI)

[not] [but nevertheless interesting]

> I don't think either approach would solve your question though.
> 

In any case, what i wanted to convey, but failed, is that this
doesn't, in my opinion, belong to the CODING_STYLE.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[Blue Swirl]

On Fri, Aug 13, 2010 at 9:56 PM, malc <av1474@comtv.ru> wrote:
> On Fri, 13 Aug 2010, Blue Swirl wrote:
>
>> On Fri, Aug 13, 2010 at 9:01 PM, malc <av1474@comtv.ru> wrote:
>> > On Fri, 13 Aug 2010, Blue Swirl wrote:
>> >
>> >> On Thu, Aug 12, 2010 at 5:50 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>> >> > Add memory management rules, somewhat like libvirt HACKING.
>> >> >
>> >> > Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>> >> > ---
>> >> >  CODING_STYLE |    8 ++++++++
>> >> >  1 files changed, 8 insertions(+), 0 deletions(-)
>> >> >
>> >> > diff --git a/CODING_STYLE b/CODING_STYLE
>> >> > index 3f10d72..085c86f 100644
>> >> > --- a/CODING_STYLE
>> >> > +++ b/CODING_STYLE
>> >> > @@ -148,3 +148,11 @@ up-front that this is a read-only pointer.  Perhaps more
>> >> >  importantly, if we're diligent about this, when you see a non-const
>> >> >  pointer, you're guaranteed that it is used to modify the storage
>> >> >  it points to, or it is aliased to another pointer that is.
>> >> > +
>> >> > +
>> >> > +7. Low level memory management
>> >> > +
>> >> > +Use of the malloc/free/realloc/calloc APIs is not allowed in the QEMU
>> >>
>> >> I forgot to add valloc/memalign/posix_memalign.
>> >>
>> >> > +codebase. Instead of these routines, use the replacement
>> >> > +qemu_malloc/qemu_mallocz/qemu_realloc/qemu_free or
>> >> > +qemu_vmalloc/qemu_memalign/qemu_vfree APIs.
>> >>
>> >> This should also mention that memory allocated by qemu_vmalloc or
>> >> qemu_memalign should be freed with qemu_vfree.
>> >>
>> >> In general, whole of 7 is an existing rule and current codebase seems
>> >> to follow it.
>> >>
>> >> This should be added as a strict rule (as opposed to guideline), since
>> >> breaking this will cause problems on Win32 and user emulators.
>> >>
>> >
>> > This is all perfectly reasonable, but begs one question, where do we stop
>> > codifying self-evident truths (or who is the ultimiate judge of what is
>> > self-evident and what isn't), since taking the above to extreme we will
>> > end up adding stuff like: you must use close to dispose of open-ed
>> > descriptors and such like.
>>
>> Interesting question. We could assume that the target audience knows
>> well how to write portable C, conformal to various standards, so we
>> only have to point out QEMU specific pitfalls. This approach would
>> probably mean that there would be no need to mention, for example, the
>> issues with identifiers starting with an underscore.
>>
>> Another approach is to consider the history. Which types of problems
>> there have been frequently with the proposed patches? Then the
>> underscore issue (just as an example) should definitely be raised,
>> even though that is just one aspect of standards conformance.
>
> I.e. having a (N)FAQ(BNI)
>
> [not] [but nevertheless interesting]
>
>> I don't think either approach would solve your question though.
>>
>
> In any case, what i wanted to convey, but failed, is that this
> doesn't, in my opinion, belong to the CODING_STYLE.

Well, this is not coding style but architectural guide. We could
introduce a new document, rename CODING_STYLE or bury this to
qemu-tech.texi. But I still (optimistically) think that some kind of
READ_ME_BEFORE_CODING would help.

Re: [Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[Andreas Färber]

Am 14.08.2010 um 12:02 schrieb Blue Swirl:

> On Fri, Aug 13, 2010 at 9:56 PM, malc <av1474@comtv.ru> wrote:
>> In any case, what i wanted to convey, but failed, is that this
>> doesn't, in my opinion, belong to the CODING_STYLE.
>
> Well, this is not coding style but architectural guide. We could
> introduce a new document, rename CODING_STYLE or bury this to
> qemu-tech.texi. But I still (optimistically) think that some kind of
> READ_ME_BEFORE_CODING would help.

I agree that we shouldn't overload CODING_STYLE.

qemu-tech.texi historically was more a description of how QEMU works.
Is there a reason not to simply put this FAQ/guide into the new Wiki?
Otherwise we could stick with HACKING as file name.

Andreas

[Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[Paolo Bonzini]

On 08/14/2010 10:52 AM, Andreas Färber wrote:
>>
>> Well, this is not coding style but architectural guide. We could
>> introduce a new document, rename CODING_STYLE or bury this to
>> qemu-tech.texi. But I still (optimistically) think that some kind of
>> READ_ME_BEFORE_CODING would help.
>
> I agree that we shouldn't overload CODING_STYLE.
>
> qemu-tech.texi historically was more a description of how QEMU works.
> Is there a reason not to simply put this FAQ/guide into the new Wiki?
> Otherwise we could stick with HACKING as file name.

If HACKING is chosen, it would anyway contain CODING_STYLE.  I don't 
have any preference as long as the content is kept in only one file. 
Actually, I have a preference for CODING_STYLE since I see no reason to 
rename a file (also considering git's rename tracking is not perfect).

Paolo

Re: [Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules

[Avi Kivity]

  On 08/14/2010 12:01 AM, malc wrote:
>
> This is all perfectly reasonable, but begs one question, where do we stop
> codifying self-evident truths (or who is the ultimiate judge of what is
> self-evident and what isn't), since taking the above to extreme we will
> end up adding stuff like: you must use close to dispose of open-ed
> descriptors and such like.

It's a matter of pragmatism, if a lot of patches are routinely rejected 
due to some error then it makes sense to add it to CODING_STYLE, even if 
they are not exactly style decisions.  If not then they just dilute that 
document.

-- 
error compiling committee.c: too many arguments to function