qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Blue Swirl <blauwirbel@gmail.com>
To: malc <av1474@comtv.ru>
Cc: qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] Re: [PATCH 3/5] CODING_STYLE: add memory management rules
Date: Sat, 14 Aug 2010 10:02:58 +0000	[thread overview]
Message-ID: <AANLkTimTe3-Jb5O2uLJxZ7dUKFX-W_NUJoRGmeb1DSNz@mail.gmail.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1008140150190.8405@linmac>

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.

  reply	other threads:[~2010-08-14 10:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-12 17:50 [Qemu-devel] [PATCH 3/5] CODING_STYLE: add memory management rules Blue Swirl
2010-08-13 19:24 ` [Qemu-devel] " Blue Swirl
2010-08-13 21:01   ` malc
2010-08-13 21:28     ` Blue Swirl
2010-08-13 21:56       ` malc
2010-08-14 10:02         ` Blue Swirl [this message]
2010-08-14 14:52           ` Andreas Färber
2010-08-15 14:07             ` Paolo Bonzini
2010-08-15 16:06     ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTimTe3-Jb5O2uLJxZ7dUKFX-W_NUJoRGmeb1DSNz@mail.gmail.com \
    --to=blauwirbel@gmail.com \
    --cc=av1474@comtv.ru \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).