From: Junio C Hamano Date: Thu, 05 Apr 2007 05:14:12 GMT Subject: Re: [PATCH 1/2] Added use of xmalloc() on diff-delta.c Message-ID: <7vy7l7wggr.fsf@assigned-by-dhcp.cox.net> In-Reply-To: Linus Torvalds writes: > On Wed, 4 Apr 2007, Junio C Hamano wrote: >> >> These patches take that nice property away, making libification >> more difficult, which is the downside. Is there an upside? > > Well, we could just make the libification rule very simple: > > - the library does *not* include "xmalloc()", and you have to handle > out-of-memory situations yourself inside the xmalloc() that *you* as a > libification user provide!. > > Then, we just make our xmalloc() be non-inlined (which we should do > *anyway* - it's long since grown so big that it shouldn't be inlined in > the first place), and we make it part of a non-library git object file. I agree that is probably a sane thing to do for existing callers of xmalloc() -- the callers are not prepared to handle (near-)oom case gracefully to begin with. Your "libified git decides how xmalloc() copes with (near-)oom condition gracefully" is much better than "memory-allocating function in git declares that oom in xmalloc() is fatal", which is what we currently have. I however think it is an independent issue, wrt the part Bruno's patch touches. In the case of this particular call chain between create_delta()/create_delta_index() and its caller, I think the code that is there allows nicer arrangement. The caller could instead easily attempt to cope with (near-)oom condition more gracefully. And that was my suggestion about returning 0 when delta_index cannot be built instead of dying. Of course, this caller is in memory-hungry "pack-objects", and all of the above is mostly academic, as failure to allocate memory in create_delta_index() would most likely mean you would have trouble allocating memory for other more important data.