[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] libxl: Check for regcomp failure

Eric Blake wrote:
> On 09/04/2013 02:03 PM, Jim Fehlig wrote:
>>> Yeah, good question.  I found a few occurrences of regcomp() and friends
>>> throughout the sources and most seem to do regfree() even when regcomp()
>>> fails.  The man page is not very clear, but the notes on regfree()
>>> suggest it is not necessary
>>>   POSIX Pattern Buffer Freeing
>>>     Supplying regfree() with a precompiled pattern buffer, preg will
>>>     free the memory allocated to the pattern buffer by the compiling
>>>     process, regcomp().
>>> But does the pattern buffer contain any allocated memory when regcomp()
>>> fails?  The notes on regcomp() are not clear about this.
> Thankfully, we can read the source :)

Nod :).  Was about to do that before seeing your message...

> In glibc, regcomp assigns into preg, but is careful to undo any
> allocation on failure; it is also careful to make regfree() a no-op on
> an already-freed buffer (whether by calling regfree() twice in a row, or
> using it on preg after a failed regcomp).  Gnulib copies this behavior.
>  But it is not universally standard:
>> The System Interfaces volume of POSIX.1-2008 [1] says this about
>> regcomp() return value
>> Upon successful completion, the regcomp() function shall return 0.
>> Otherwise, it shall return an integer value indicating an error as
>> described in /<regex.h>/
>> <http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/regex.h.html>,
>> and the content of preg is undefined. If a code is returned, the
>> interpretation shall be as given in /<regex.h>/
>> <http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/regex.h.html>.
>> I don't think we want to call regfree() on an undefined preg right?
> Correct - regfree() is only needed on successful regcomp().  We can
> probably get away with calling regfree() even on failure because we use
> gnulib, but that's not a good reason, so it wouldn't hurt to audit the
> code and guarantee a free only on success.

I've pushed this patch, given Michal's ACK before raising the regfree()

Also sent a small series to remove other unnecessary uses of regfree()



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]