Merge struct etherinfo and etherinfo_py

Make things more "pythonic" and avoid another layer of wrapping
by removing the struct etherinfo.  Move that information to the
main Python object instead.

This simplifies the object creation and handling too, as now
all strings are python objects.

Also update some of the documentation blocks along the way.

Signed-off-by: David Sommerseth <davids@redhat.com>

Author: David Sommerseth

python-ethtool/etherinfo.c

@@ -41,23 +41,6 @@
  *
  */
 
-/**
- * Frees the memory used by struct etherinfo
- *
- * @param ptr Pointer to a struct etherninfo element
- */
-void free_etherinfo(struct etherinfo *ptr)
-{
-	if( ptr == NULL ) { // Just for safety
-		return;
-	}
-
-        Py_XDECREF(ptr->device);
-        Py_XDECREF(ptr->hwaddress);
-
-	free(ptr);
-}
-
 
 /**
  *  libnl callback function.  Does the real parsing of a record returned by NETLINK.  This function
@@ -68,7 +51,7 @@ void free_etherinfo(struct etherinfo *ptr)
  */
 static void callback_nl_link(struct nl_object *obj, void *arg)
 {
-	struct etherinfo *ethi = (struct etherinfo *) arg;
+	etherinfo_py *ethi = (etherinfo_py *) arg;
 	struct rtnl_link *link = (struct rtnl_link *) obj;
 	char hwaddr[130];
 
@@ -125,12 +108,12 @@ static void callback_nl_address(struct nl_object *obj, void *arg)
 /**
  * Sets the etherinfo.index member to the corresponding device set in etherinfo.device
  *
- * @param ethinf A pointer a struct etherinfo element which contains the device name
- *               and a place to save the corresponding index value.
+ * @param self A pointer the current etherinfo_py Python object which contains the device name
+ *               and the place where to save the corresponding index value.
  *
  * @return Returns 1 on success, otherwise 0.
  */
-static int _set_device_index(struct etherinfo *ethinf)
+static int _set_device_index(etherinfo_py *self)
 {
 	struct nl_cache *link_cache;
 	struct rtnl_link *link;
@@ -139,19 +122,19 @@ static int _set_device_index(struct etherinfo *ethinf)
 	 * As we don't expect it to change, we're reusing a "cached"
 	 * interface index if we have that
 	 */
-	if( ethinf->index < 0 ) {
+	if( self->index < 0 ) {
 		if( rtnl_link_alloc_cache(get_nlc(), AF_UNSPEC, &link_cache) < 0) {
                         return 0;
                 }
 
-                link = rtnl_link_get_by_name(link_cache, PyString_AsString(ethinf->device));
+                link = rtnl_link_get_by_name(link_cache, PyString_AsString(self->device));
                 if( !link ) {
 			nl_cache_free(link_cache);
                         return 0;
                 }
 
-		ethinf->index = rtnl_link_get_ifindex(link);
-		if( ethinf->index < 0 ) {
+		self->index = rtnl_link_get_ifindex(link);
+		if( self->index < 0 ) {
 			rtnl_link_put(link);
 			nl_cache_free(link_cache);
 			return 0;
@@ -169,26 +152,31 @@ static int _set_device_index(struct etherinfo *ethinf)
  *
  */
 
+/**
+ * Populate the etherinfo_py Python object with link information for the current device
+ *
+ * @param self  Pointer to the device object, a etherinfo_py Python object
+ *
+ * @return Returns 1 on success, otherwise 0
+ */
 int get_etherinfo_link(etherinfo_py *self)
 {
 	struct nl_cache *link_cache;
 	struct rtnl_link *link;
-	struct etherinfo *ethinf = NULL;
 
-	if( !self || !self->ethinfo ) {
+	if( !self ) {
 		return 0;
 	}
-	ethinf = self->ethinfo;
 
 	/* Open a NETLINK connection on-the-fly */
 	if( !open_netlink(self) ) {
 		PyErr_Format(PyExc_RuntimeError,
 			     "Could not open a NETLINK connection for %s",
-			     PyString_AsString(ethinf->device));
+			     PyString_AsString(self->device));
 		return 0;
 	}
 
-        if( _set_device_index(ethinf) != 1) {
+        if( _set_device_index(self) != 1) {
                 return 0;
         }
 
@@ -198,8 +186,8 @@ int get_etherinfo_link(etherinfo_py *self)
         }
         link = rtnl_link_alloc();
         /* FIXME: Error handling? */
-        rtnl_link_set_ifindex(link, ethinf->index);
-        nl_cache_foreach_filter(link_cache, OBJ_CAST(link), callback_nl_link, ethinf);
+        rtnl_link_set_ifindex(link, self->index);
+        nl_cache_foreach_filter(link_cache, OBJ_CAST(link), callback_nl_link, self);
         rtnl_link_put(link);
         nl_cache_free(link_cache);
 
@@ -211,34 +199,32 @@ int get_etherinfo_link(etherinfo_py *self)
 /**
  * Query NETLINK for device IP address configuration
  *
- * @param ethinf Pointer to an available struct etherinfo element.  The 'device' member
- *               must contain a valid string to the device to query for information
- * @param nlc    Pointer to the libnl handle, which is used for the query against NETLINK
- * @param query  What to query for.  Must be NLQRY_ADDR4 or NLQRY_ADDR6.
+ * @param self   A etherinfo_py Python object for the current device to retrieve IP address
+ *               configuration data from
+ * @param query  What to query for.  Must be NLQRY_ADDR4 for IPv4 addresses or NLQRY_ADDR6
+ *               for IPv6 addresses.
  *
  * @return Returns a Python list containing PyNetlinkIPaddress objects on success, otherwise NULL
  */
 PyObject * get_etherinfo_address(etherinfo_py *self, nlQuery query)
 {
 	struct nl_cache *addr_cache;
 	struct rtnl_addr *addr;
-	struct etherinfo *ethinf = NULL;
         PyObject *addrlist = NULL;
 
-	if( !self || !self->ethinfo ) {
+	if( !self ) {
 		return NULL;
 	}
-	ethinf = self->ethinfo;
 
 	/* Open a NETLINK connection on-the-fly */
 	if( !open_netlink(self) ) {
 		PyErr_Format(PyExc_RuntimeError,
 			     "Could not open a NETLINK connection for %s",
-			     PyString_AsString(ethinf->device));
+			     PyString_AsString(self->device));
 		return NULL;
 	}
 
-        if( _set_device_index(ethinf) != 1) {
+        if( _set_device_index(self) != 1) {
                 return NULL;
         }
 
@@ -251,7 +237,7 @@ PyObject * get_etherinfo_address(etherinfo_py *self, nlQuery query)
         }
         addr = rtnl_addr_alloc();
         /* FIXME: Error handling? */
-        rtnl_addr_set_ifindex(addr, ethinf->index);
+        rtnl_addr_set_ifindex(addr, self->index);
 
 	switch( query ) {
         case NLQRY_ADDR4:

python-ethtool/etherinfo.h

@@ -21,7 +21,6 @@ typedef enum {NLQRY_ADDR4, NLQRY_ADDR6} nlQuery; /**<  Supported query types in
 
 int get_etherinfo_link(etherinfo_py *data);
 PyObject * get_etherinfo_address(etherinfo_py *self, nlQuery query);
-void free_etherinfo(struct etherinfo *ptr);
 
 int open_netlink(etherinfo_py *);
 struct nl_sock * get_nlc();

python-ethtool/etherinfo_obj.c

@@ -34,58 +34,17 @@
 /**
  * ethtool.etherinfo deallocator - cleans up when a object is deleted
  *
- * @param self etherinfo_py object structure
+ * @param self etherinfo_py Python object to deallocate
  */
-void _ethtool_etherinfo_dealloc(etherinfo_py *self)
+static void _ethtool_etherinfo_dealloc(etherinfo_py *self)
 {
 	close_netlink(self);
-	if( self->ethinfo ) {
-                free_etherinfo(self->ethinfo);
-	}
+        Py_XDECREF(self->device);    self->device = NULL;
+        Py_XDECREF(self->hwaddress); self->hwaddress = NULL;
 	self->ob_type->tp_free((PyObject*)self);
 }
 
 
-/**
- * ethtool.etherinfo function, creating a new etherinfo object
- *
- * @param type
- * @param args
- * @param kwds
- *
- * @return Returns in PyObject with the new object on success, otherwise NULL
- */
-PyObject *_ethtool_etherinfo_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
-{
-	etherinfo_py *self;
-
-	self = (etherinfo_py *)type->tp_alloc(type, 0);
-	return (PyObject *)self;
-}
-
-
-/**
- * ethtool.etherinfo init (constructor) method.  Makes sure the object is initialised correctly.
- *
- * @param self
- * @param args
- * @param kwds
- *
- * @return Returns 0 on success.
- */
-int _ethtool_etherinfo_init(etherinfo_py *self, PyObject *args, PyObject *kwds)
-{
-	static char *etherinfo_kwlist[] = {"etherinfo_ptr", NULL};
-	PyObject *ethinf_ptr = NULL;
-
-	if( !PyArg_ParseTupleAndKeywords(args, kwds, "O", etherinfo_kwlist, &ethinf_ptr)) {
-		PyErr_SetString(PyExc_AttributeError, "Invalid data pointer to constructor");
-		return -1;
-	}
-	self->ethinfo = (struct etherinfo *) PyCObject_AsVoidPtr(ethinf_ptr);
-	return 0;
-}
-
 /*
   The old approach of having a single IPv4 address per device meant each result
   that came in from netlink overwrote the old result.
@@ -120,7 +79,7 @@ static PyNetlinkIPaddress * get_last_ipv4_address(PyObject *addrlist)
 /**
  * ethtool.etherinfo function for retrieving data from a Python object.
  *
- * @param self
+ * @param self    Pointer to the current etherinfo_py 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
@@ -131,22 +90,22 @@ PyObject *_ethtool_etherinfo_getter(etherinfo_py *self, PyObject *attr_o)
 	PyNetlinkIPaddress *py_addr;
         PyObject *addrlist = NULL;
 
-	if( !self || !self->ethinfo ) {
+	if( !self ) {
 		PyErr_SetString(PyExc_AttributeError, "No data available");
 		return NULL;
 	}
 
 	if( strcmp(attr, "device") == 0 ) {
-                if( self->ethinfo->device ) {
-                        Py_INCREF(self->ethinfo->device);
-                        return self->ethinfo->device;
+                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->ethinfo->hwaddress);
-		return self->ethinfo->hwaddress;
+		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: */
@@ -200,7 +159,7 @@ int _ethtool_etherinfo_setter(etherinfo_py *self, PyObject *attr_o, PyObject *va
 /**
  * Creates a human readable format of the information when object is being treated as a string
  *
- * @param self
+ * @param self  Pointer to the current etherinfo_py device object
  *
  * @return Returns a PyObject with a string with all of the information
  */
@@ -209,20 +168,20 @@ PyObject *_ethtool_etherinfo_str(etherinfo_py *self)
 	PyObject *ret = NULL;
         PyObject *ipv4addrs = NULL, *ipv6addrs = NULL;
 
-	if( !self || !self->ethinfo ) {
+	if( !self ) {
 		PyErr_SetString(PyExc_AttributeError, "No data available");
 		return NULL;
 	}
 
 	get_etherinfo_link(self);
 
 	ret = PyString_FromFormat("Device ");
-	PyString_Concat(&ret, self->ethinfo->device);
+	PyString_Concat(&ret, self->device);
 	PyString_ConcatAndDel(&ret, PyString_FromString(":\n"));
 
-	if( self->ethinfo->hwaddress ) {
+	if( self->hwaddress ) {
 		PyString_ConcatAndDel(&ret, PyString_FromString("\tMAC address: "));
-		PyString_Concat(&ret, self->ethinfo->hwaddress);
+		PyString_Concat(&ret, self->hwaddress);
 		PyString_ConcatAndDel(&ret, PyString_FromString("\n"));
 	}
 
@@ -265,13 +224,13 @@ PyObject *_ethtool_etherinfo_str(etherinfo_py *self)
 /**
  * Returns a tuple list of configured IPv4 addresses
  *
- * @param self
+ * @param self     Pointer to the current etherinfo_py device object to extract IPv4 info from
  * @param notused
  *
  * @return Returns a Python tuple list of NetlinkIP4Address objects
  */
 static PyObject *_ethtool_etherinfo_get_ipv4_addresses(etherinfo_py *self, PyObject *notused) {
-	if( !self || !self->ethinfo ) {
+	if( !self ) {
 		PyErr_SetString(PyExc_AttributeError, "No data available");
 		return NULL;
 	}
@@ -281,15 +240,15 @@ static PyObject *_ethtool_etherinfo_get_ipv4_addresses(etherinfo_py *self, PyObj
 
 
 /**
- * Returns a tuple list of configured IPv4 addresses
+ * Returns a tuple list of configured IPv6 addresses
  *
- * @param self
+ * @param self     Pointer to the current etherinfo_py device object to extract IPv6 info from
  * @param notused
  *
  * @return Returns a Python tuple list of NetlinkIP6Address objects
  */
 static PyObject *_ethtool_etherinfo_get_ipv6_addresses(etherinfo_py *self, PyObject *notused) {
-	if( !self || !self->ethinfo ) {
+	if( !self ) {
 		PyErr_SetString(PyExc_AttributeError, "No data available");
 		return NULL;
 	}
@@ -351,8 +310,8 @@ PyTypeObject ethtool_etherinfoType = {
     0,                         /* tp_descr_get */
     0,                         /* tp_descr_set */
     0,                         /* tp_dictoffset */
-    (initproc)_ethtool_etherinfo_init,     /* tp_init */
+    0,     /* tp_init */
     0,                         /* tp_alloc */
-    _ethtool_etherinfo_new,                /* tp_new */
+    0,                /* tp_new */
 };
 

python-ethtool/etherinfo_struct.h

@@ -28,16 +28,6 @@
 
 #include <netlink/route/addr.h>
 
-/**
- * Contains IP address information about a particular ethernet device
- *
- */
-struct etherinfo {
-	PyObject *device;                   /**< Device name */
-	int index;                          /**< NETLINK index reference */
-	PyObject *hwaddress;             /**< string: HW address / MAC address of device */
-};
-
 /* Python object containing data baked from a (struct rtnl_addr) */
 typedef struct PyNetlinkIPaddress {
 	PyObject_HEAD
@@ -56,8 +46,10 @@ extern PyTypeObject ethtool_netlink_ip_address_Type;
  */
 typedef struct {
 	PyObject_HEAD
-	struct etherinfo *ethinfo;      /**< Information about the interface configuration */
-	unsigned short nlc_active;	/**< Is this instance using NETLINK? */
+	PyObject *device;                   /**< Device name */
+	int index;                          /**< NETLINK index reference */
+	PyObject *hwaddress;                /**< string: HW address / MAC address of device */
+	unsigned short nlc_active;	    /**< Is this instance using NETLINK? */
 } etherinfo_py;