[PATCH] Do away with the generic getattribute and setattribute of PyEtherInfo

This is a simplification patch before I submit again the patches for adding 
extra information

Antoni S. Puimedon (1):
  Do away with the generic getattribute and setattribute of PyEtherInfo

 python-ethtool/etherinfo_obj.c | 173 +++++++++++++++++++++--------------------
 python-ethtool/ethtool.c       |   8 +-
 2 files changed, 96 insertions(+), 85 deletions(-)

-- 
1.9.0

index and mac_address are etherinfo attributes and as such it is best to
report them as members of the objects and benefit from Python member
retrieving infrastructure (it also helps discoverability in ipython).

ipv4_address, ipv4_netmask and ipv4_broadcast are best set in isolated
methods referred by a getsetter that simplify the code and help
discoverability and documentation.

Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com&gt;
---
 python-ethtool/etherinfo_obj.c | 173 +++++++++++++++++++++--------------------
 python-ethtool/ethtool.c       |   8 +-
 2 files changed, 96 insertions(+), 85 deletions(-)

diff --git a/python-ethtool/etherinfo_obj.c b/python-ethtool/etherinfo_obj.c
index f620157..b7f68e7 100644
--- a/python-ethtool/etherinfo_obj.c
+++ b/python-ethtool/etherinfo_obj.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2013 Red Hat Inc.
+ * Copyright (C) 2009-2014 Red Hat Inc.
  *
  * David Sommerseth <davids(a)redhat.com&gt;
  *
@@ -76,85 +76,6 @@ static PyNetlinkIPaddress * get_last_ipv4_address(PyObject *addrlist)
 	return NULL;
 }
 
-/**
- * ethtool.etherinfo function for retrieving data from a Python object.
- *
- * @param self    Pointer to the current PyEtherInfo device object
- * @param attr_o  contains the object member request (which element to return)
- *
- * @return Returns a PyObject with the value requested on success, otherwise NULL
- */
-PyObject *_ethtool_etherinfo_getter(PyEtherInfo *self, PyObject *attr_o)
-{
-	char *attr = PyString_AsString(attr_o);
-	PyNetlinkIPaddress *py_addr;
-        PyObject *addrlist = NULL;
-
-	if( !self ) {
-		PyErr_SetString(PyExc_AttributeError, "No data available");
-		return NULL;
-	}
-
-	if( strcmp(attr, "device") == 0 ) {
-                if( self->device ) {
-                        Py_INCREF(self->device);
-                        return self->device;
-                } else {
-                        return Py_INCREF(Py_None), Py_None;
-                }
-	} else if( strcmp(attr, "mac_address") == 0 ) {
-		get_etherinfo_link(self);
-		Py_INCREF(self->hwaddress);
-		return self->hwaddress;
-	} else if( strcmp(attr, "ipv4_address") == 0 ) {
-		addrlist = get_etherinfo_address(self, NLQRY_ADDR4);
-		/* For compatiblity with old approach, return last IPv4 address: */
-		py_addr = get_last_ipv4_address(addrlist);
-		if (py_addr) {
-		  if (py_addr->local) {
-		      Py_INCREF(py_addr->local);
-		      return py_addr->local;
-		  }
-		}
-		Py_RETURN_NONE;
-	} else if( strcmp(attr, "ipv4_netmask") == 0 ) {
-		addrlist = get_etherinfo_address(self, NLQRY_ADDR4);
-		py_addr = get_last_ipv4_address(addrlist);
-		if (py_addr) {
-		  return PyInt_FromLong(py_addr->prefixlen);
-		}
-		return PyInt_FromLong(0);
-	} else if( strcmp(attr, "ipv4_broadcast") == 0 ) {
-		addrlist = get_etherinfo_address(self, NLQRY_ADDR4);
-		py_addr = get_last_ipv4_address(addrlist);
-		if (py_addr) {
-		  if (py_addr->ipv4_broadcast) {
-		      Py_INCREF(py_addr->ipv4_broadcast);
-		      return py_addr->ipv4_broadcast;
-		  }
-		}
-		Py_RETURN_NONE;
-	} else {
-		return PyObject_GenericGetAttr((PyObject *)self, attr_o);
-	}
-}
-
-/**
- * ethtool.etherinfo function for setting a value to a object member.  This feature is
- * disabled by always returning -1, as the values are read-only by the user.
- *
- * @param self
- * @param attr_o
- * @param val_o
- *
- * @return Returns always -1 (failure).
- */
-int _ethtool_etherinfo_setter(PyEtherInfo *self, PyObject *attr_o, PyObject *val_o)
-{
-	PyErr_SetString(PyExc_AttributeError, "etherinfo member values are
read-only.");
-	return -1;
-}
-
 
 /**
  * Creates a human readable format of the information when object is being treated as a
string
@@ -173,8 +94,6 @@ PyObject *_ethtool_etherinfo_str(PyEtherInfo *self)
 		return NULL;
 	}
 
-	get_etherinfo_link(self);
-
 	ret = PyString_FromFormat("Device ");
 	PyString_Concat(&ret, self->device);
 	PyString_ConcatAndDel(&ret, PyString_FromString(":\n"));
@@ -258,6 +177,66 @@ static PyObject *_ethtool_etherinfo_get_ipv6_addresses(PyEtherInfo
*self, PyObje
 
 
 /**
+ * Returns the last ipv4 address (due to compatibility with older clients).
+ *
+ * @param self	Pointer to the current PyEtherInfo device object
+ * @param closure	Not used
+ *
+ * @return Returns a PyString if it has an address, otherwise None.
+ */
+static PyObject *_etherinfo_get_ipv4_address(PyEtherInfo *self, void *closure) {
+	PyObject *addrlist = get_etherinfo_address(self, NLQRY_ADDR4);
+	PyNetlinkIPaddress *py_addr = get_last_ipv4_address(addrlist);
+	if (py_addr) {
+		if (py_addr->local) {
+			Py_INCREF(py_addr->local);
+			return py_addr->local;
+		}
+	}
+	Py_RETURN_NONE;
+}
+
+/**
+ * Returns the last ipv4 netmask (due to compatibility with older clients).
+ *
+ * @param self	Pointer to the current PyEtherInfo device object
+ * @param closure	Not used
+ *
+ * @return Returns the netmask prefix as a PyInt or 0 if there is no address.
+ */
+static PyObject *_etherinfo_get_ipv4_netmask(PyEtherInfo *self, void *closure) {
+	PyObject *addrlist = get_etherinfo_address(self, NLQRY_ADDR4);
+	PyNetlinkIPaddress * py_addr = get_last_ipv4_address(addrlist);
+	if (py_addr) {
+		return PyInt_FromLong(py_addr->prefixlen);
+	}
+	return PyInt_FromLong(0);
+}
+
+
+/**
+ * Returns the last ipv4 broadcast address (due to compatibility with older clients).
+ *
+ * @param self	Pointer to the current PyEtherInfo device object
+ * @param closure	Not used
+ *
+ * @return Returns a PyString if it has an address, otherwise None.
+ */
+static PyObject *_etherinfo_get_ipv4_broadcast(PyEtherInfo *self, void *closure) {
+	PyObject *addrlist = get_etherinfo_address(self, NLQRY_ADDR4);
+	PyNetlinkIPaddress *py_addr = get_last_ipv4_address(addrlist);
+	if (py_addr) {
+		if (py_addr->ipv4_broadcast) {
+			Py_INCREF(py_addr->ipv4_broadcast);
+			return py_addr->ipv4_broadcast;
+		}
+	}
+	Py_RETURN_NONE;
+}
+
+
+
+/**
  * Defines all available methods in the ethtool.etherinfo class
  *
  */
@@ -269,6 +248,32 @@ static PyMethodDef _ethtool_etherinfo_methods[] = {
 	{NULL}  /**< No methods defined */
 };
 
+static PyMemberDef _ethtool_etherinfo_members[] = {
+    {"device", T_OBJECT_EX, offsetof(PyEtherInfo, device), READONLY,
+     "Name of the device"},
+    {"mac_address", T_OBJECT_EX, offsetof(PyEtherInfo, hwaddress), READONLY,
+     "MAC address of the device"},
+    {"index", T_INT, offsetof(PyEtherInfo, index), READONLY,
+     "Numeric index of the device"},
+    {NULL}  /* Sentinel */
+};
+
+static PyGetSetDef _ethtool_etherinfo_getsetters[] = {
+    {"ipv4_address", 
+     (getter)_etherinfo_get_ipv4_address, NULL,
+     "IPv4 address of the device (deprecated)",
+     NULL},
+    {"ipv4_netmask", 
+     (getter)_etherinfo_get_ipv4_netmask, NULL,
+     "IPv4 netmask of the device (deprecated)",
+     NULL},
+    {"ipv4_broadcast", 
+     (getter)_etherinfo_get_ipv4_broadcast, NULL,
+     "IPv4 broadcast address of the device (deprecated)",
+     NULL},
+    {NULL}  /* Sentinel */
+}; 
+
 /**
  * Definition of the functions a Python class/object requires.
  *
@@ -280,9 +285,9 @@ PyTypeObject PyEtherInfo_Type = {
     .tp_flags = Py_TPFLAGS_HAVE_CLASS,
     .tp_dealloc = (destructor)_ethtool_etherinfo_dealloc,
     .tp_str = (reprfunc)_ethtool_etherinfo_str,
-    .tp_getattro = (getattrofunc)_ethtool_etherinfo_getter,
-    .tp_setattro = (setattrofunc)_ethtool_etherinfo_setter,
+    .tp_getset = _ethtool_etherinfo_getsetters,
     .tp_methods = _ethtool_etherinfo_methods,
+    .tp_members = _ethtool_etherinfo_members,
     .tp_doc = "Contains information about a specific ethernet device"
 };
 
diff --git a/python-ethtool/ethtool.c b/python-ethtool/ethtool.c
index 0f9cdbb..3e9d593 100644
--- a/python-ethtool/ethtool.c
+++ b/python-ethtool/ethtool.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2013 Red Hat Inc.
+ * Copyright (C) 2008-2014 Red Hat Inc.
  *
  * Arnaldo Carvalho de Melo <acme(a)redhat.com&gt;
  * David Sommerseth <davids(a)redhat.com&gt;
@@ -268,8 +268,14 @@ static PyObject *get_interfaces_info(PyObject *self __unused,
PyObject *args) {
 			return NULL;
                 }
 		dev->device = PyString_FromString(fetch_devs[i]);
+		/* Set pre-initialization values and get link info */
 		dev->hwaddress = NULL;
 		dev->index = -1;
+		int success = get_etherinfo_link(dev);
+		if( !success) {
+			free(fetch_devs);
+			return NULL;
+		}
 
 		/* Append device object to the device list */
 		PyList_Append(devlist, (PyObject *)dev);
-- 
1.9.0