On Wed, 26 Jul 2023 at 16:21, Richard Henderson <richard.hender...@linaro.org> wrote: > > On 7/26/23 02:43, Peter Maydell wrote: > > (Something went wrong with the quoting in your email. I've > > fixed it up.) > > > > On Wed, 26 Jul 2023 at 05:38, <dingli...@cmss.chinamobile.com> wrote: > >> Peter Maydell wrote: > >>> The third part here, is that g_malloc() does not ever > >>> fail -- it will abort() on out of memory. However > >>> the code here is still handling g_malloc() returning NULL. > >>> The equivalent for "we expect this might fail" (which we want > >>> here, because the guest is passing us the length of memory > >>> to try to allocate) is g_try_malloc(). > > > >> g_malloc() is preferred more than g_try_* functions, which return NULL on > >> error, > >> when the size of the requested allocation is small. > >> This is because allocating few bytes should not be a problem in a healthy > >> system. > > > > This is true. But in this particular case we cannot be sure > > that the size of the allocation is small, because the size > > is controlled by the guest. So we want g_try_malloc(). > > And why do we want to use g_try_malloc instead of just sticking with malloc?
The only real reason is just consistency -- the project uses the glib malloc wrappers, and in theory any use of raw malloc() ought to be either: * something that's third party library code (eg libdecnumber) * because it's going into a standalone program that doesn't link against glib * for a special case reason which is documented in a nearby comment (eg because the memory is going to be passed to some library which documents that it will assume it can free() the memory) We're down to very few uses of malloc() that don't fall into one of those cases: bsd-user/elfload.c a few uses in disas/ (m68k, sparc, nios2) hw/audio/fmopl.c semihosting/uaccess.c target/xtensa/translate.c contrib/elf2dmp/ (and maybe tests/qtest/fuzz/qtest_wrappers.c : I'm not sure what environment that gets built in.) The main reason we get patches like this is that the "bite sized tasks" list at https://wiki.qemu.org/Contribute/BiteSizedTasks has an entry for "convert malloc uses to g_new or similar". The trouble with that is that almost all of the low-hanging fruit was converted a long time ago, so the remainder are getting increasingly tricky to analyse and increasingly less worthwhile... thanks -- PMM
