From: Brandon Casey Date: Thu, 06 Oct 2011 16:14:23 GMT Subject: Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere Message-ID: In-Reply-To: <4E8D4812.9090102@viscovery.net> [removed Alexey Shumkin from cc] On Thu, Oct 6, 2011 at 1:17 AM, Johannes Sixt wrote: > 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 wrote: >>> On Thursday, September 15, 2011, Brandon Casey wrote: >>>> >>>> On Thu, Sep 15, 2011 at 1:52 AM, Johannes Sixt >>>>> 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. Ah. Yes, you're right. x-wrappers should not be used in syslog.c and the use of strbuf's should be replaced. Thanks, -Brandon