Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere
- From
Johannes Sixt <j.sixt@viscovery.net>
- Date
- Oct 6, 2011, 06:17 UTC
- Message-ID
- <4E8D4812.9090102@viscovery.net>
- In-Reply-To
- <CA+sFfMdHpvdMU==a2sUR9sZZCcgqPfGF7+dy6yi8RVoMZ+uZVA@mail.gmail.com>
Am 10/6/2011 4:00, schrieb Brandon Casey:
Show 24 quoted lines
> [resend without html bits added by "gmail offline"] > On Wed, Oct 5, 2011 at 7:53 PM, Brandon Casey <drafnel@gmail.com> wrote: >> On Thursday, September 15, 2011, Brandon Casey wrote: >>> >>> On Thu, Sep 15, 2011 at 1:52 AM, Johannes Sixt <j.sixt@viscovery.net> >>>> 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