[Spice-devel] [PATCH spice-server v2 00/23] Use GLib memory allocation

[Christophe Fergeau]

On Wed, Sep 20, 2017 at 05:31:37PM +0200, Christophe de Dinechin wrote:
> 
> 
> > On 20 Sep 2017, at 16:51, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > 
> > On Wed, Sep 20, 2017 at 02:54:31PM +0200, Christophe de Dinechin wrote:
> >>> 
> >>>> The benefit of doing it that way (in addition to requiring less source code
> >>>> changes and making following rebases or merge much easier) is that it leaves
> >>>> the option to instrument spice allocations specifically when the need
> >>>> arises.
> >>>> 
> >>> 
> >>> There are many tools to instruments memory allocations and is not hard
> >>> to write one on your own. For instance knowing that objects file takes
> >>> precedence over libraries you can write a module defining malloc, or use
> >>> --wrap linker option or LD_PRELOAD.
> >> 
> >> That works if you want to instrument all malloc calls. If you want to do
> >> something specific to spice, you can’t do that.
> > 
> > You could do that with systemtap for example. And I really don't think
> > we should have our spice_xxx wrappers for library calls.
> 
> But then, we don’t need g_xxx wrappers either, do we?

They (both g_malloc and spice_malloc) abort on OOM, straight malloc does not.

> 
> Anyway, if we were starting to write the code, I’d agree. But here, we do a big patch
> just to remove the *existing* wrapper, including changes at practically all call sites.
> That means a future rebase is going to show potential conflicts all over the place.
> For everybody with branches that touch anything close to one of these call sites.
> 
> So the question is: how does putting a macro wrapper not solve the
> alleged problem with having a wrapper, whatever that problem is, without
> requiring all call sites to be modified?

I don't understand what you are asking in the last paragraph. If you are
having an issue with all the churn this patch series causes (the rebase
problem you mention in the paragraph before the last), that's a fair
point, and I agree that's a concern. But I don't think this is what you
were discussing before this email.

Christophe