Re: [PATCH 1/2] Added use of xmalloc() on diff-delta.c

Linus Torvalds [off-list ref] writes:

On Wed, 4 Apr 2007, Junio C Hamano wrote:

quoted

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.