On 06/12/14 02:46, Eric Blake wrote: > On 12/05/2014 06:23 PM, Pádraig Brady wrote: >> * lib/vasnprintf.c (VASNPRINTF): Fix free-memory read, >> flagged by clang-analyzer 3.4.2. >> --- >> ChangeLog | 6 ++++++ >> lib/vasnprintf.c | 2 +- >> 2 files changed, 7 insertions(+), 1 deletion(-) >> > >> +++ b/lib/vasnprintf.c >> @@ -5184,13 +5184,13 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, >> free (result); >> if (buf_malloced != NULL) >> free (buf_malloced); >> - CLEANUP (); >> errno = >> (saved_errno != 0 >> ? saved_errno >> : (dp->conversion == 'c' || dp->conversion == 's' >> ? EILSEQ >> : EINVAL)); >> + CLEANUP (); > > Ouch. This is a bug. The whole point of assigning to errno after > CLEANUP() is that CLEANUP() may invalidate the value stored in errno. > > I suggest doing something like: > > if (saved_errno == 0) > saved_errno = dp->conversion == 'c' || dp->conversion == 's' > ? EILSEQ : EINVAL; > CLEANUP(); > errno = saved_errno;
Oh right, I did think about it and looked at: http://austingroupbugs.net/view.php?id=385 but jumped to the wrong conclusion that the updated standard was just documenting existing practise. thanks, Pádraig
