Thomas Huth <th...@redhat.com> writes:
> On 15/03/2021 07.43, Mahmoud Mandour wrote: >> If it's unrelated, then maybe better do it in a separate patch. >> I thought so but I didn't know whether it was a so-small change >> that it didn't require its own patch or not. I will amend that. >> Since this is only a very small allocation, I think it would be >> better to >> use g_malloc() here and then simply remove the "if (info == NULL) ..." >> part. >> I was thinking of always maintaining the semantics of the existing >> code and since g_malloc() does not behave like malloc() on >> error, I refrained from using g_malloc() anywhere, but of course >> I'll do it since it's the better thing to do. > > Keeping the semantics is normally a good idea, but the common sense in > the QEMU project is to rather use g_malloc() for small allocations (if > allocating some few bytes already fails, then the system is pretty > much dead anyway), and only g_try_malloc() for huge allocations that > really might fail on a healthy system, too. > > We should likely add some text to our coding style document to make > this more obvious... So while there are some places where we may try to dynamically scale the memory we allocate on failure of a large allocation generally memory allocation failure is considered fatal (ergo g_malloc, no NULL check). However some care has to be taken depending on where we are - for example calling abort() because something the guest did triggered us to try an allocate more memory than we could is a no no. We could certainly be clearer in style.rst though. >> I will split the patches to a two-patch series regarding the >> util/compactfd.c file (one for the style change and one for >> changing the malloc() call into g_malloc()) and send them >> again, is that ok? > > Sounds good, thanks! > > Thomas -- Alex Bennée
