Re: [PATCH] libxl: Check for regcomp failure

[Jim Fehlig <jfehlig@xxxxxxxx>]

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@xxxxxxxx>
>>> ---
>>>  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(&regex,
>>> +                      "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
>>> +                       REG_NOSUB | REG_EXTENDED)) != 0) {
>>> +        char error[100];
>>> +        regerror(res, &regex, 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(&regex);
> 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.

Regards,
Jim

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list