These two functions, create_delta_index() and create_delta(),
are already nicely libified. They allow the caller to deal with
oom condition. The caller may die(), or it may decide to
continue its operation with reduced functionality without using
delta data. A good example of this is found a few lines after
the lines the second patch touches. When create_delta() cannot
find memory to work with, the entire function returns 0, saying
"sorry, cannot deltify these two", which would cause the object
stored without deltification.
These patches take that nice property away, making libification
more difficult, which is the downside. Is there an upside?
If anything, I suspect that the part that calls die() you
touched in the second patch could return NULL.
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.
Other libgit uses might end up doing something like
..
if (sigsetjump(buffer, 1)) {
show_oom_message();
..
void *xmalloc(size_t size)
{
void *ret = malloc(size ? size : 1);
if (!ret)
siglongjmp(buffer);
return ret;
}
or, if they use C++ exception handling, they'd just make their own
xmalloc() raise an exception, and have the callers catch it.
The point being that this is what you'd need to do *anyway*, and trying to
make all the library routines return NULL or some other error case is just
worse programming practice than just having a xmalloc() that dies by
default but that can be overridden.
Linus