Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere

Am 10/6/2011 4:00, schrieb Brandon Casey:

[resend without html bits added by "gmail offline"]
On Wed, Oct 5, 2011 at 7:53 PM, Brandon Casey [off-list ref] wrote:

quoted

On Thursday, September 15, 2011, Brandon Casey wrote:

quoted

On Thu, Sep 15, 2011 at 1:52 AM, Johannes Sixt [off-list ref]

quoted

There is a danger that the high-level die() routine (which is used by
the
x-wrappers) uses one of the low-level compat/ routines. IOW, in the case
of errors, recursion might occur. Therefore, I would prefer that the
compat/ routines do their own error reporting (preferably via return
values and errno).

Thanks.  Will do.

Hi Johannes,
I have taken a closer look at the possibility of recursion with respect to
die() and the functions in compat/.  It appears the risk is only related to
vsnprintf/snprintf at the moment.  So as long as we avoid calling xmalloc et
al from within snprintf.c, I think we should be safe from recursion.
I'm inclined to keep the additions to mingw.c and win32/syslog.c since they
both already use the x-wrappers or strbuf, even though they could easily be
worked around.  The other file that was touched is compat/qsort, which
returns void and doesn't have a good alternative error handling path.  So,
I'm inclined to keep that one too.

I'm fine with keeping the change to mingw.c (getaddrinfo related) and
qsort: both are unlikely to be called from die().

But syslog() *is* called from die() in git-daemon, and it would be better
to back out the other offenders instead of adding to them.

-- Hannes