A virNodeDeviceObjPtr is an @obj

A virNodeDeviceObjListPtr is an @devs

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/virnodedeviceobj.c | 130 ++++++++++++++++++++++----------------------
 1 file changed, 65 insertions(+), 65 deletions(-)

diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 913cdda..a2d09ad 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -34,10 +34,10 @@ VIR_LOG_INIT("conf.virnodedeviceobj");
 
 
 static int
-virNodeDeviceObjHasCap(const virNodeDeviceObj *dev,
+virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
                        const char *cap)
 {
-    virNodeDevCapsDefPtr caps = dev->def->caps;
+    virNodeDevCapsDefPtr caps = obj->def->caps;
     const char *fc_host_cap =
         virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST);
     const char *vports_cap =
@@ -100,9 +100,9 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *dev,
  * Pointer to the caps or NULL if not found
  */
 static virNodeDevCapsDefPtr
-virNodeDeviceFindFCCapDef(const virNodeDeviceObj *dev)
+virNodeDeviceFindFCCapDef(const virNodeDeviceObj *obj)
 {
-    virNodeDevCapsDefPtr caps = dev->def->caps;
+    virNodeDevCapsDefPtr caps = obj->def->caps;
 
     while (caps) {
         if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST &&
@@ -124,9 +124,9 @@ virNodeDeviceFindFCCapDef(const virNodeDeviceObj *dev)
  * Pointer to the caps or NULL if not found
  */
 static virNodeDevCapsDefPtr
-virNodeDeviceFindVPORTCapDef(const virNodeDeviceObj *dev)
+virNodeDeviceFindVPORTCapDef(const virNodeDeviceObj *obj)
 {
-    virNodeDevCapsDefPtr caps = dev->def->caps;
+    virNodeDevCapsDefPtr caps = obj->def->caps;
 
     while (caps) {
         if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST &&
@@ -235,18 +235,18 @@ virNodeDeviceFindByCap(virNodeDeviceObjListPtr devs,
 
 
 void
-virNodeDeviceObjFree(virNodeDeviceObjPtr dev)
+virNodeDeviceObjFree(virNodeDeviceObjPtr obj)
 {
-    if (!dev)
+    if (!obj)
         return;
 
-    virNodeDeviceDefFree(dev->def);
-    if (dev->privateFree)
-        (*dev->privateFree)(dev->privateData);
+    virNodeDeviceDefFree(obj->def);
+    if (obj->privateFree)
+        (*obj->privateFree)(obj->privateData);
 
-    virMutexDestroy(&dev->lock);
+    virMutexDestroy(&obj->lock);
 
-    VIR_FREE(dev);
+    VIR_FREE(obj);
 }
 
 
@@ -265,51 +265,51 @@ virNodeDeviceObjPtr
 virNodeDeviceObjAssignDef(virNodeDeviceObjListPtr devs,
                           virNodeDeviceDefPtr def)
 {
-    virNodeDeviceObjPtr device;
+    virNodeDeviceObjPtr obj;
 
-    if ((device = virNodeDeviceObjFindByName(devs, def->name))) {
-        virNodeDeviceDefFree(device->def);
-        device->def = def;
-        return device;
+    if ((obj = virNodeDeviceObjFindByName(devs, def->name))) {
+        virNodeDeviceDefFree(obj->def);
+        obj->def = def;
+        return obj;
     }
 
-    if (VIR_ALLOC(device) < 0)
+    if (VIR_ALLOC(obj) < 0)
         return NULL;
 
-    if (virMutexInit(&device->lock) < 0) {
+    if (virMutexInit(&obj->lock) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("cannot initialize mutex"));
-        VIR_FREE(device);
+        VIR_FREE(obj);
         return NULL;
     }
-    virNodeDeviceObjLock(device);
+    virNodeDeviceObjLock(obj);
 
-    if (VIR_APPEND_ELEMENT_COPY(devs->objs, devs->count, device) < 0) {
-        virNodeDeviceObjUnlock(device);
-        virNodeDeviceObjFree(device);
+    if (VIR_APPEND_ELEMENT_COPY(devs->objs, devs->count, obj) < 0) {
+        virNodeDeviceObjUnlock(obj);
+        virNodeDeviceObjFree(obj);
         return NULL;
     }
-    device->def = def;
+    obj->def = def;
 
-    return device;
+    return obj;
 
 }
 
 
 void
 virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                       virNodeDeviceObjPtr *dev)
+                       virNodeDeviceObjPtr *obj)
 {
     size_t i;
 
-    virNodeDeviceObjUnlock(*dev);
+    virNodeDeviceObjUnlock(*obj);
 
     for (i = 0; i < devs->count; i++) {
         virNodeDeviceObjLock(devs->objs[i]);
-        if (devs->objs[i] == *dev) {
+        if (devs->objs[i] == *obj) {
             virNodeDeviceObjUnlock(devs->objs[i]);
             virNodeDeviceObjFree(devs->objs[i]);
-            *dev = NULL;
+            *obj = NULL;
 
             VIR_DELETE_ELEMENT(devs->objs, i, devs->count);
             break;
@@ -332,15 +332,15 @@ virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
  *   parent_host value on success (>= 0), -1 otherwise.
  */
 static int
-virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr parent)
+virNodeDeviceFindFCParentHost(virNodeDeviceObjPtr obj)
 {
-    virNodeDevCapsDefPtr cap = virNodeDeviceFindVPORTCapDef(parent);
+    virNodeDevCapsDefPtr cap = virNodeDeviceFindVPORTCapDef(obj);
 
     if (!cap) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Parent device %s is not capable "
                          "of vport operations"),
-                       parent->def->name);
+                       obj->def->name);
         return -1;
     }
 
@@ -353,19 +353,19 @@ virNodeDeviceGetParentHostByParent(virNodeDeviceObjListPtr devs,
                                    const char *dev_name,
                                    const char *parent_name)
 {
-    virNodeDeviceObjPtr parent = NULL;
+    virNodeDeviceObjPtr obj = NULL;
     int ret;
 
-    if (!(parent = virNodeDeviceObjFindByName(devs, parent_name))) {
+    if (!(obj = virNodeDeviceObjFindByName(devs, parent_name))) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Could not find parent device for '%s'"),
                        dev_name);
         return -1;
     }
 
-    ret = virNodeDeviceFindFCParentHost(parent);
+    ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(parent);
+    virNodeDeviceObjUnlock(obj);
 
     return ret;
 }
@@ -377,19 +377,19 @@ virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
                                  const char *parent_wwnn,
                                  const char *parent_wwpn)
 {
-    virNodeDeviceObjPtr parent = NULL;
+    virNodeDeviceObjPtr obj = NULL;
     int ret;
 
-    if (!(parent = virNodeDeviceFindByWWNs(devs, parent_wwnn, parent_wwpn))) {
+    if (!(obj = virNodeDeviceFindByWWNs(devs, parent_wwnn, parent_wwpn))) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Could not find parent device for '%s'"),
                        dev_name);
         return -1;
     }
 
-    ret = virNodeDeviceFindFCParentHost(parent);
+    ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(parent);
+    virNodeDeviceObjUnlock(obj);
 
     return ret;
 }
@@ -400,19 +400,19 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
                                       const char *dev_name,
                                       const char *parent_fabric_wwn)
 {
-    virNodeDeviceObjPtr parent = NULL;
+    virNodeDeviceObjPtr obj = NULL;
     int ret;
 
-    if (!(parent = virNodeDeviceFindByFabricWWN(devs, parent_fabric_wwn))) {
+    if (!(obj = virNodeDeviceFindByFabricWWN(devs, parent_fabric_wwn))) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Could not find parent device for '%s'"),
                        dev_name);
         return -1;
     }
 
-    ret = virNodeDeviceFindFCParentHost(parent);
+    ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(parent);
+    virNodeDeviceObjUnlock(obj);
 
     return ret;
 }
@@ -421,19 +421,19 @@ virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
 static int
 virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs)
 {
-    virNodeDeviceObjPtr parent = NULL;
+    virNodeDeviceObjPtr obj = NULL;
     const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
     int ret;
 
-    if (!(parent = virNodeDeviceFindByCap(devs, cap))) {
+    if (!(obj = virNodeDeviceFindByCap(devs, cap))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Could not find any vport capable device"));
         return -1;
     }
 
-    ret = virNodeDeviceFindFCParentHost(parent);
+    ret = virNodeDeviceFindFCParentHost(obj);
 
-    virNodeDeviceObjUnlock(parent);
+    virNodeDeviceObjUnlock(obj);
 
     return ret;
 }
@@ -481,12 +481,12 @@ virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj)
 
 
 static bool
-virNodeDeviceCapMatch(virNodeDeviceObjPtr devobj,
+virNodeDeviceCapMatch(virNodeDeviceObjPtr obj,
                       int type)
 {
     virNodeDevCapsDefPtr cap = NULL;
 
-    for (cap = devobj->def->caps; cap; cap = cap->next) {
+    for (cap = obj->def->caps; cap; cap = cap->next) {
         if (type == cap->data.type)
             return true;
 
@@ -589,9 +589,9 @@ virNodeDeviceObjGetNames(virNodeDeviceObjListPtr devs,
 
 
 #define MATCH(FLAG) ((flags & (VIR_CONNECT_LIST_NODE_DEVICES_CAP_ ## FLAG)) && \
-                     virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_ ## FLAG))
+                     virNodeDeviceCapMatch(obj, VIR_NODE_DEV_CAP_ ## FLAG))
 static bool
-virNodeDeviceMatch(virNodeDeviceObjPtr devobj,
+virNodeDeviceMatch(virNodeDeviceObjPtr obj,
                    unsigned int flags)
 {
     /* filter by cap type */
@@ -621,7 +621,7 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj,
 
 int
 virNodeDeviceObjListExport(virConnectPtr conn,
-                           virNodeDeviceObjListPtr devobjs,
+                           virNodeDeviceObjListPtr devs,
                            virNodeDevicePtr **devices,
                            virNodeDeviceObjListFilter filter,
                            unsigned int flags)
@@ -632,26 +632,26 @@ virNodeDeviceObjListExport(virConnectPtr conn,
     int ret = -1;
     size_t i;
 
-    if (devices && VIR_ALLOC_N(tmp_devices, devobjs->count + 1) < 0)
+    if (devices && VIR_ALLOC_N(tmp_devices, devs->count + 1) < 0)
         goto cleanup;
 
-    for (i = 0; i < devobjs->count; i++) {
-        virNodeDeviceObjPtr devobj = devobjs->objs[i];
-        virNodeDeviceObjLock(devobj);
-        if ((!filter || filter(conn, devobj->def)) &&
-            virNodeDeviceMatch(devobj, flags)) {
+    for (i = 0; i < devs->count; i++) {
+        virNodeDeviceObjPtr obj = devs->objs[i];
+        virNodeDeviceObjLock(obj);
+        if ((!filter || filter(conn, obj->def)) &&
+            virNodeDeviceMatch(obj, flags)) {
             if (devices) {
-                if (!(device = virGetNodeDevice(conn, devobj->def->name)) ||
-                    VIR_STRDUP(device->parent, devobj->def->parent) < 0) {
+                if (!(device = virGetNodeDevice(conn, obj->def->name)) ||
+                    VIR_STRDUP(device->parent, obj->def->parent) < 0) {
                     virObjectUnref(device);
-                    virNodeDeviceObjUnlock(devobj);
+                    virNodeDeviceObjUnlock(obj);
                     goto cleanup;
                 }
                 tmp_devices[ndevices] = device;
             }
             ndevices++;
         }
-        virNodeDeviceObjUnlock(devobj);
+        virNodeDeviceObjUnlock(obj);
     }
 
     if (tmp_devices) {
-- 
2.9.4

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

On Thu, May 25, 2017 at 15:57:02 -0400, John Ferlan wrote:
> A virNodeDeviceObjPtr is an @obj
> 
> A virNodeDeviceObjListPtr is an @devs
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/conf/virnodedeviceobj.c | 130 ++++++++++++++++++++++----------------------
>  1 file changed, 65 insertions(+), 65 deletions(-)
> 
> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index 913cdda..a2d09ad 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -34,10 +34,10 @@ VIR_LOG_INIT("conf.virnodedeviceobj");
>  
>  
>  static int
> -virNodeDeviceObjHasCap(const virNodeDeviceObj *dev,
> +virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,

I'm not a fan of this. Adds line churn and it's not really worth it.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 05/26/2017 03:15 AM, Peter Krempa wrote:
> On Thu, May 25, 2017 at 15:57:02 -0400, John Ferlan wrote:
>> A virNodeDeviceObjPtr is an @obj
>>
>> A virNodeDeviceObjListPtr is an @devs
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>  src/conf/virnodedeviceobj.c | 130 ++++++++++++++++++++++----------------------
>>  1 file changed, 65 insertions(+), 65 deletions(-)
>>
>> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
>> index 913cdda..a2d09ad 100644
>> --- a/src/conf/virnodedeviceobj.c
>> +++ b/src/conf/virnodedeviceobj.c
>> @@ -34,10 +34,10 @@ VIR_LOG_INIT("conf.virnodedeviceobj");
>>  
>>  
>>  static int
>> -virNodeDeviceObjHasCap(const virNodeDeviceObj *dev,
>> +virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
> 
> I'm not a fan of this. Adds line churn and it's not really worth it.
> 

Undo-ing would alter so many patches going forward which causes such a
ripple effect w/r/t the process I've been using for all these patches
and a "common" look and feel.

I get the churn and back port thing - really, but I was asked to undo
the single monolithic patch and this is what happens - (too) many
smaller patches each that can be debated and affect the final result.
So, is it technically wrong to rename or is it just not being a fan?
Just because I'm not a fan of something someone else has done, doesn't
make it wrong.

While one could focus on a module by module approach, for me it's been a
bit more global. In some code the vir<driver>Ptr is referenced as an
@obj while other times it's referenced as @<drv>.  In the same module
there's vir<driver>ObjPtr's that are sometimes referenced as @obj and
other times as @<drv>.  (where <drv> is some shorthand, dev, iface, net,
filter, secret, pool, vol, etc.).

In this module an @device is a virNodeDeviceObjPtr in
virNodeDeviceObjAssignDef and a virNodeDevicePtr in
virNodeDeviceObjListExport. So without going back to the argument
definition how does one really know what it is. If one creates a common
way to describe arguments, then one doesn't have to think about it. So
that's been my approach.

>From my perspective of trying to create this "common" look and feel to
arguments across all these various modules and to help me keep some
amount of sanity, I've tried to think of vir<driver>ObjPtr as @obj and
vir<driver>Ptr as @drv.

Of course there's also the vir<driver>ObjListPtr's that as was pointed
out could be changed into @objs (following the same logic); however, for
me that was too close to @obj, so I left those as devs, nets, nwfilters,
pools, ifaces, etc. (in general).

John

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