[libvirt] [PATCH] libxl: Check for regcomp failure
Jim Fehlig
jfehlig at suse.com
Wed Sep 4 20:03:47 UTC 2013
Jim Fehlig wrote:
> Michal Privoznik wrote:
>
>> On 04.09.2013 10:29, Michal Privoznik wrote:
>>
>>
>>> On 04.09.2013 08:37, Jim Fehlig wrote:
>>>
>>>
>>>> Change libxlGetAutoballoonConf() function to return an int
>>>> for success/failure, and fail if regcomp fails.
>>>>
>>>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>>>> ---
>>>> src/libxl/libxl_conf.c | 30 +++++++++++++++++++-----------
>>>> 1 file changed, 19 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>>>> index fcb278b..a634476 100644
>>>> --- a/src/libxl/libxl_conf.c
>>>> +++ b/src/libxl/libxl_conf.c
>>>> @@ -1014,21 +1014,28 @@ error:
>>>> return -1;
>>>> }
>>>>
>>>> -static bool
>>>> -libxlGetAutoballoonConf(libxlDriverConfigPtr cfg)
>>>> +static int
>>>> +libxlGetAutoballoonConf(libxlDriverConfigPtr cfg, bool *autoballoon)
>>>> {
>>>> regex_t regex;
>>>> - int ret;
>>>> + int res;
>>>> +
>>>> + if ((res = regcomp(®ex,
>>>> + "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
>>>> + REG_NOSUB | REG_EXTENDED)) != 0) {
>>>> + char error[100];
>>>> + regerror(res, ®ex, error, sizeof(error));
>>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>>> + _("Failed to compile regex %s"),
>>>> + error);
>>>>
>>>>
>>> My only concern is, that 100 bytes may not be enough sometimes. But on
>>> the other hand, there's another occurrence of regerror and there's
>>> buffer of the exact size passed too. So I'm comfortable enough to give
>>> you my ACK.
>>>
>>>
>>>
>> Hit send prematurely, the other occurrence is doing regfree(®ex);
>> even on error path (in fact, just on the error path). I must confess I
>> don't get it (in case of error, there should not be any regex created,
>> should it?). So - do we need to regfree() even on error?
>>
>>
>
> 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.
>
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?
Regards,
Jim
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/regcomp.html
More information about the libvir-list
mailing list