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

Re: [libvirt] [PATCHv2 01/11] don't modify CPU set string in virDomainCpuSetParse



On 11/18/2011 03:35 AM, Daniel P. Berrange wrote:
> On Thu, Nov 17, 2011 at 05:44:11PM +0800, Hu Tao wrote:
>> As the function only parses the CPU set string, there is
>> no good reason to modify it.
>> ---
>>  src/conf/domain_conf.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 6b78d97..71eb209 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -9163,7 +9163,6 @@ virDomainCpuSetParse(const char **str, char sep,
>>          } else
>>              goto parse_error;
>>      }
>> -    *str = cur;
>>      return (ret);
>>  
>>    parse_error:
> 
> ACK

NACK.  I audited all existing callers to ensure that they really didn't
care if str wasn't updated; xend_internal.c:sexpr_to_xend_topology() was
the trickiest, but I'm convinced it was safe (you might get one more
iteration of the outer while(*cur != 0) loop than previously, but no one
used the modified cur before anyone else modified cur again in a manner
that would work from either the old or the new position).

Better would be to modify the function signature, adjust all callers (as
proof that our audit was sane), and as a side-effect, get rid of some
nasty casting.  Alternative v3 patch coming up.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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