Re: [PATCH] libxl: Check for regcomp failure

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

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?

Michal

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