On Fri, Mar 06, 2015 at 09:42:24 +0100, Peter Krempa wrote:
On Thu, Mar 05, 2015 at 12:05:18 +0100, Michal Privoznik wrote:
Every API that touches internal structure of the object must lock the object first. Not every API that has the object as an argument needs to do that though. Some APIs just pass the object to lower layers which, however, must lock the object then. Look at the code, you'll get my meaning soon.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/network_conf.c | 46 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 8cf9ffd..7af303e 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -157,16 +157,19 @@ virNetworkObjPtr virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets, const unsigned char *uuid) { + virNetworkObjPtr ret = NULL; size_t i;
for (i = 0; i < nets->count; i++) { virObjectLock(nets->objs[i]); - if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) - return nets->objs[i]; + if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) { + ret = nets->objs[i]; + break; + } virObjectUnlock(nets->objs[i]); }
This hunk ...
- return NULL; + return ret; }
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets, @@ -184,16 +187,19 @@ virNetworkObjPtr virNetworkObjFindByNameLocked(virNetworkObjListPtr nets, const char *name) { + virNetworkObjPtr ret = NULL; size_t i;
for (i = 0; i < nets->count; i++) { virObjectLock(nets->objs[i]); - if (STREQ(nets->objs[i]->def->name, name)) - return nets->objs[i]; + if (STREQ(nets->objs[i]->def->name, name)) { + ret = nets->objs[i]; + break; + } virObjectUnlock(nets->objs[i]); }
and this hunk have no semantic impact. I presume they are an artifact of the changes from previous version. I'd appreciate if you'd remove them but I won't insist.
They make sense in the next patch though ... so they are just misplaced. I don't think it's worth moving them though. Peter