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@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list