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 <address@hidden> wrote:
> On Fri, 13 Aug 2010, Blue Swirl wrote:
>
>> On Thu, Aug 12, 2010 at 5:50 PM, Blue Swirl <address@hidden> wrote:
>> > Add memory management rules, somewhat like libvirt HACKING.
>> >
>> > Signed-off-by: Blue Swirl <address@hidden>
>> > ---
>> >  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.