qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: [PATCH v2 03/22] docs/devel: expand style section of memory management
Date: Tue, 23 Mar 2021 16:52:49 +0000	[thread overview]
Message-ID: <20210323165308.15244-4-alex.bennee@linaro.org> (raw)
In-Reply-To: <20210323165308.15244-1-alex.bennee@linaro.org>

This aims to provide a bit more guidance for those who take on one of
our "clean up memory allocation" bite-sized tasks.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210320133706.21475-4-alex.bennee@linaro.org>
---
 docs/devel/style.rst | 46 ++++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index 8b0bdb3570..260e3263fa 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -385,17 +385,37 @@ avoided.
 Low level memory management
 ===========================
 
-Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
+Use of the ``malloc/free/realloc/calloc/valloc/memalign/posix_memalign``
 APIs is not allowed in the QEMU codebase. Instead of these routines,
-use the GLib memory allocation routines g_malloc/g_malloc0/g_new/
-g_new0/g_realloc/g_free or QEMU's qemu_memalign/qemu_blockalign/qemu_vfree
-APIs.
-
-Please note that g_malloc will exit on allocation failure, so there
-is no need to test for failure (as you would have to with malloc).
-Calling g_malloc with a zero size is valid and will return NULL.
-
-Prefer g_new(T, n) instead of g_malloc(sizeof(T) ``*`` n) for the following
+use the GLib memory allocation routines
+``g_malloc/g_malloc0/g_new/g_new0/g_realloc/g_free``
+or QEMU's ``qemu_memalign/qemu_blockalign/qemu_vfree`` APIs.
+
+Please note that ``g_malloc`` will exit on allocation failure, so
+there is no need to test for failure (as you would have to with
+``malloc``). Generally using ``g_malloc`` on start-up is fine as the
+result of a failure to allocate memory is going to be a fatal exit
+anyway. There may be some start-up cases where failing is unreasonable
+(for example speculatively loading a large debug symbol table).
+
+Care should be taken to avoid introducing places where the guest could
+trigger an exit by causing a large allocation. For small allocations,
+of the order of 4k, a failure to allocate is likely indicative of an
+overloaded host and allowing ``g_malloc`` to ``exit`` is a reasonable
+approach. However for larger allocations where we could realistically
+fall-back to a smaller one if need be we should use functions like
+``g_try_new`` and check the result. For example this is valid approach
+for a time/space trade-off like ``tlb_mmu_resize_locked`` in the
+SoftMMU TLB code.
+
+If the lifetime of the allocation is within the function and there are
+multiple exist paths you can also improve the readability of the code
+by using ``g_autofree`` and related annotations. See :ref:`autofree-ref`
+for more details.
+
+Calling ``g_malloc`` with a zero size is valid and will return NULL.
+
+Prefer ``g_new(T, n)`` instead of ``g_malloc(sizeof(T) * n)`` for the following
 reasons:
 
 * It catches multiplication overflowing size_t;
@@ -409,8 +429,8 @@ Declarations like
 
 are acceptable, though.
 
-Memory allocated by qemu_memalign or qemu_blockalign must be freed with
-qemu_vfree, since breaking this will cause problems on Win32.
+Memory allocated by ``qemu_memalign`` or ``qemu_blockalign`` must be freed with
+``qemu_vfree``, since breaking this will cause problems on Win32.
 
 String manipulation
 ===================
@@ -485,6 +505,8 @@ In addition, QEMU assumes that the compiler does not use the latitude
 given in C99 and C11 to treat aspects of signed '<<' as undefined, as
 documented in the GNU Compiler Collection manual starting at version 4.0.
 
+.. _autofree-ref:
+
 Automatic memory deallocation
 =============================
 
-- 
2.20.1



  parent reply	other threads:[~2021-03-23 17:42 UTC|newest]

Thread overview: 149+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23 16:52 [PATCH for 6.0 v2 00/22] fixes for rc1 pre-PR (kernel-doc, semihosting, testing) Alex Bennée
2021-03-23 16:52 ` [PATCH v2 01/22] scripts/kernel-doc: strip QEMU_ from function definitions Alex Bennée
2021-03-23 16:52 ` [PATCH v2 02/22] docs/devel: include the plugin API information from the headers Alex Bennée
2021-03-23 16:52 ` Alex Bennée [this message]
2021-03-23 16:52 ` [PATCH v2 04/22] tools/virtiofsd: include --socket-group in help Alex Bennée
2021-03-23 16:52 ` [PATCH v2 05/22] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-23 16:52 ` [PATCH v2 06/22] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-23 16:52 ` [PATCH v2 07/22] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-23 16:52   ` [Bug 1915925] " Alex Bennée
2021-03-23 16:52 ` [PATCH v2 08/22] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-23 21:29   ` Alistair Francis
2021-03-23 16:52 ` [PATCH v2 09/22] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-24  6:11   ` Thomas Huth
2021-03-24 14:09   ` Richard Henderson
2021-03-23 16:52 ` [PATCH v2 10/22] gitlab-ci.yml: Merge the trace-backend testing into other jobs Alex Bennée
2021-03-23 16:52 ` [PATCH v2 11/22] configure: Don't use the __atomic_*_16 functions for testing 128-bit support Alex Bennée
2021-03-23 16:52 ` [PATCH v2 12/22] cirrus.yml: Update the FreeBSD task to version 12.2 Alex Bennée
2021-03-23 16:52 ` [PATCH v2 13/22] utils: Tighter tests for qemu_strtosz Alex Bennée
2021-03-23 16:53 ` [PATCH v2 14/22] utils: Work around mingw strto*l bug with 0x Alex Bennée
2021-03-23 16:53 ` [PATCH v2 15/22] gitlab: extend timeouts for CFI builds Alex Bennée
2021-03-23 16:53 ` [PATCH v2 16/22] qdev: define list of archs with virtio-pci or virtio-ccw Alex Bennée
2021-03-23 16:53 ` [PATCH v2 17/22] m68k: add the virtio devices aliases Alex Bennée
2021-03-23 16:53 ` [PATCH v2 18/22] blockdev: with -drive if=virtio, use generic virtio-blk Alex Bennée
2021-03-23 16:53 ` [PATCH v2 19/22] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182" Alex Bennée
2021-03-23 16:53 ` [PATCH v2 20/22] iotests: test m68k with the virt machine Alex Bennée
2021-03-23 16:53 ` [PATCH v2 21/22] iotests: iothreads need ioeventfd Alex Bennée
2021-03-23 16:53 ` [PATCH v2 22/22] gitlab: default to not building the documentation Alex Bennée
2021-03-23 18:21 ` [PATCH for 6.0 v2 00/22] fixes for rc1 pre-PR (kernel-doc, semihosting, testing) no-reply
2021-03-24 13:40 ` Peter Maydell
2021-03-24 14:22   ` Alex Bennée
2021-03-24 15:58     ` Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2021-03-24 14:29 [PULL for 6.0 00/22] various fixes " Alex Bennée
2021-03-24 14:30 ` [PULL 01/22] scripts/kernel-doc: strip QEMU_ from function definitions Alex Bennée
2021-03-24 14:30 ` [PULL 02/22] docs/devel: include the plugin API information from the headers Alex Bennée
2021-03-24 14:30 ` [PULL 03/22] docs/devel: expand style section of memory management Alex Bennée
2021-03-24 14:30 ` [PULL 04/22] tools/virtiofsd: include --socket-group in help Alex Bennée
2021-03-24 14:30 ` [PULL 05/22] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-24 14:30 ` [PULL 06/22] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-24 14:30 ` [PULL 07/22] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-24 14:30   ` [Bug 1915925] " Alex Bennée
2021-03-24 14:30 ` [PULL 08/22] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-24 14:30 ` [PULL 09/22] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-24 14:30 ` [PULL 10/22] gitlab-ci.yml: Merge the trace-backend testing into other jobs Alex Bennée
2021-03-24 14:30 ` [PULL 11/22] configure: Don't use the __atomic_*_16 functions for testing 128-bit support Alex Bennée
2021-03-24 14:30 ` [PULL 12/22] cirrus.yml: Update the FreeBSD task to version 12.2 Alex Bennée
2021-03-24 14:30 ` [PULL 13/22] utils: Tighter tests for qemu_strtosz Alex Bennée
2021-03-24 14:30 ` [PULL 14/22] utils: Work around mingw strto*l bug with 0x Alex Bennée
2021-03-24 14:30 ` [PULL 15/22] gitlab: extend timeouts for CFI builds Alex Bennée
2021-03-24 14:30 ` [PULL 16/22] qdev: define list of archs with virtio-pci or virtio-ccw Alex Bennée
2021-03-24 14:30 ` [PULL 17/22] m68k: add the virtio devices aliases Alex Bennée
2021-03-24 14:30 ` [PULL 18/22] blockdev: with -drive if=virtio, use generic virtio-blk Alex Bennée
2021-03-24 14:30 ` [PULL 19/22] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182" Alex Bennée
2021-03-24 14:30 ` [PULL 20/22] iotests: test m68k with the virt machine Alex Bennée
2021-03-24 14:30 ` [PULL 21/22] iotests: iothreads need ioeventfd Alex Bennée
2021-03-24 14:30 ` [PULL 22/22] gitlab: default to not building the documentation Alex Bennée
2021-03-24 14:57 ` [PULL for 6.0 00/22] various fixes (kernel-doc, semihosting, testing) no-reply
2021-03-24 17:41 ` Peter Maydell
2021-03-20 13:36 [PATCH for 6.0 v1 00/14] fixes for rc1 " Alex Bennée
2021-03-20 13:36 ` [PATCH v1 01/14] scripts/kernel-doc: strip QEMU_ from function definitions Alex Bennée
2021-03-20 16:04   ` Richard Henderson
2021-03-20 13:36 ` [PATCH v1 02/14] docs/devel: include the plugin API information from the headers Alex Bennée
2021-03-20 13:36 ` [PATCH v1 03/14] docs/devel: expand style section of memory management Alex Bennée
2021-03-20 16:00   ` Richard Henderson
2021-03-20 13:36 ` [PATCH v1 04/14] tools/virtiofsd: include --socket-group in help Alex Bennée
2021-03-22 11:46   ` Stefan Hajnoczi
2021-03-20 13:36 ` [PATCH v1 05/14] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-20 16:03   ` Richard Henderson
2021-03-20 13:36 ` [PATCH v1 06/14] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-20 13:36 ` [PATCH v1 07/14] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-20 13:36   ` [Bug 1915925] " Alex Bennée
2021-03-20 13:37 ` [PATCH v1 08/14] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-20 16:05   ` Richard Henderson
2021-03-20 13:37 ` [PATCH v1 09/14] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-20 16:11   ` Richard Henderson
2021-03-20 13:37 ` [PATCH v1 10/14] gitlab-ci.yml: Merge the trace-backend testing into other jobs Alex Bennée
2021-03-22 17:03   ` Willian Rampazzo
2021-03-20 13:37 ` [PATCH v1 11/14] configure: Don't use the __atomic_*_16 functions for testing 128-bit support Alex Bennée
2021-03-20 13:37 ` [PATCH v1 12/14] cirrus.yml: Update the FreeBSD task to version 12.2 Alex Bennée
2021-03-20 13:37 ` [PATCH v1 13/14] utils: Tighter tests for qemu_strtosz Alex Bennée
2021-03-20 13:37 ` [PATCH v1 14/14] utils: Work around mingw strto*l bug with 0x Alex Bennée
2021-03-20 13:54 ` [PATCH for 6.0 v1 00/14] fixes for rc1 (kernel-doc, semihosting, testing) no-reply
2021-03-12 10:20 [PATCH v5 0/5] semihosting/next (SYS_HEAPINFO) Alex Bennée
2021-03-12 10:20 ` [PATCH v5 1/5] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-12 10:20 ` [PATCH v5 2/5] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-12 10:20 ` [PATCH v5 3/5] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-12 10:20   ` [Bug 1915925] " Alex Bennée
2021-03-12 10:32   ` Peter Maydell
2021-03-12 10:32     ` [Bug 1915925] " Peter Maydell
2021-03-12 10:20 ` [PATCH v5 4/5] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-12 10:20 ` [PATCH v5 5/5] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-12 10:35   ` Peter Maydell
2021-03-12 11:23     ` Alex Bennée
2021-03-12 11:27       ` Peter Maydell
2021-03-12 14:08         ` Alex Bennée
2021-03-09 17:21 [PATCH v3 0/4] semihosting/next (SYS_HEAPINFO) Alex Bennée
2021-03-09 17:21 ` [PATCH v3 1/4] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-09 17:21 ` [PATCH v3 2/4] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-09 17:21 ` [PATCH v3 4/4] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-11 13:32 ` [PATCH v3 0/4] semihosting/next (SYS_HEAPINFO) Peter Maydell
2021-03-11 20:05   ` Alex Bennée
2021-03-09 14:17 [PATCH v2 0/4] semihosting/next (SYS_HEAPINFO fix) Alex Bennée
2021-03-09 14:17 ` [PATCH v2 1/4] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-09 14:17 ` [PATCH v2 2/4] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-09 16:33   ` Peter Maydell
2021-03-09 17:02   ` Keith Packard via
2021-03-09 17:24     ` Alex Bennée
2021-03-09 14:17 ` [PATCH v2 3/4] semihosting/arm-compat-semi: deref parameter register for SYS_HEAPINFO Alex Bennée
2021-03-09 14:17   ` [Bug 1915925] " Alex Bennée
2021-03-09 16:35   ` [Bug 1915925] " Peter Maydell
2021-03-09 16:35     ` Peter Maydell
2021-03-09 17:01     ` Alex Bennée
2021-03-09 17:01       ` [Bug 1915925] " Alex Bennée
2021-03-09 14:17 ` [PATCH v2 4/4] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-09 17:08   ` Keith Packard via
2021-03-05 13:54 [PATCH v1 0/3] semihosting/next (move from hw, heapinfo) Alex Bennée
2021-03-05 13:54 ` [PATCH v1 1/3] semihosting: Move include/hw/semihosting/ -> include/semihosting/ Alex Bennée
2021-03-05 13:54 ` [PATCH v1 2/3] semihosting: Move hw/semihosting/ -> semihosting/ Alex Bennée
2021-03-05 13:54 ` [PATCH v1 3/3] semihosting/arg-compat: fix up handling of SYS_HEAPINFO Alex Bennée
2021-03-05 13:54   ` [Bug 1915925] " Alex Bennée
2021-03-05 14:10   ` Peter Maydell
2021-03-05 14:10     ` [Bug 1915925] " Peter Maydell
2021-03-05 20:22     ` Keith Packard via
2021-03-05 20:22       ` [Bug 1915925] " Keith Packard
2021-03-05 22:54       ` Peter Maydell
2021-03-05 22:54         ` [Bug 1915925] " Peter Maydell
2021-03-05 23:54         ` Keith Packard via
2021-03-05 23:54           ` [Bug 1915925] " Keith Packard
2021-03-06  1:27           ` Richard Henderson
2021-03-06 14:07           ` Peter Maydell
2021-03-06 14:07             ` [Bug 1915925] " Peter Maydell
2021-03-06 16:54             ` Keith Packard via
2021-03-06 16:54               ` [Bug 1915925] " Keith Packard
2021-03-08 10:09               ` Peter Maydell
2021-03-08 10:09                 ` [Bug 1915925] " Peter Maydell
2021-03-08 13:36                 ` Alistair Francis
2021-03-08 17:28                   ` Keith Packard via
2021-03-08 17:28                     ` [Bug 1915925] " Keith Packard
2021-03-05 20:19   ` Keith Packard via
2021-03-05 20:19     ` [Bug 1915925] " Keith Packard
2021-02-17 12:19 [Bug 1915925] [NEW] ARM semihosting HEAPINFO results wrote to wrong address iNvEr7
2021-02-17 14:50 ` [Bug 1915925] " Peter Maydell
2021-02-17 15:03 ` Peter Maydell
2021-02-17 16:13 ` Philippe Mathieu-Daudé
2021-03-05 13:33 ` Alex Bennée
2021-03-09 17:21 ` [Bug 1915925] [PATCH v3 3/4] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-09 17:21   ` Alex Bennée
2021-03-15 12:46 ` [Bug 1915925] Re: ARM semihosting HEAPINFO results wrote to wrong address Alex Bennée
2021-03-25 11:47 ` Alex Bennée
2021-04-30  8:54 ` Thomas Huth

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=20210323165308.15244-4-alex.bennee@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.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).