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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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.


-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]