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

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111118/0ae39c7f/attachment-0001.sig>