[PATCH 00/12] Migrate usbip-utils to libudev

[PATCH 00/12] Migrate usbip-utils to libudev

[Valentina Manea]

This patch series modifies the USB/IP userspace side (usbip-utils)
to use libudev instead of libsysfs. This change was necessary as
libsysfs is no longer maintained and we have discovered a bug that
affected USB/IP.
On the other hand, libudev is actively maintained and recommended
for interacting with sysfs.

Valentina Manea (12):
  staging: usbip: userspace: migrate usbip_bind to libudev
  staging: usbip: userspace: remove useless libsysfs includes
  staging: usbip: userspace: migrate usbip_unbind to libudev
  staging: usbip: userspace: migrate usbip_list to libudev
  staging: usbip: userspace: re-add interface information listing
  staging: usbip: userspace: add new list API
  staging: usbip: userspace: move sysfs_utils to libsrc
  staging: usbip: userspace: migrate usbip_host_driver to libudev
  staging: usbip: userspace: remove class device infrastructure in
    vhci_driver
  staging: usbip: userspace: migrate vhci_driver to libudev
  staging: usbip: userspace: remove libsysfs flag and autoconf check
  staging: usbip: userspace: update dependencies in README

 drivers/staging/usbip/userspace/README             |   4 +-
 drivers/staging/usbip/userspace/configure.ac       |  10 +-
 drivers/staging/usbip/userspace/libsrc/Makefile.am |   4 +-
 .../staging/usbip/userspace/libsrc/build_assert.h  |  40 ++
 .../staging/usbip/userspace/libsrc/check_type.h    |  64 +++
 .../staging/usbip/userspace/libsrc/container_of.h  | 109 ++++
 drivers/staging/usbip/userspace/libsrc/list.c      |  43 ++
 drivers/staging/usbip/userspace/libsrc/list.h      | 613 +++++++++++++++++++++
 .../staging/usbip/userspace/libsrc/sysfs_utils.c   |  36 ++
 .../staging/usbip/userspace/libsrc/sysfs_utils.h   |   8 +
 .../staging/usbip/userspace/libsrc/usbip_common.c  |  74 +--
 .../staging/usbip/userspace/libsrc/usbip_common.h  |  15 +-
 .../usbip/userspace/libsrc/usbip_host_driver.c     | 282 +++-------
 .../usbip/userspace/libsrc/usbip_host_driver.h     |   7 +-
 .../staging/usbip/userspace/libsrc/vhci_driver.c   | 346 ++----------
 .../staging/usbip/userspace/libsrc/vhci_driver.h   |   9 +-
 drivers/staging/usbip/userspace/src/usbip_attach.c |   1 -
 drivers/staging/usbip/userspace/src/usbip_bind.c   | 149 ++---
 drivers/staging/usbip/userspace/src/usbip_detach.c |   2 -
 drivers/staging/usbip/userspace/src/usbip_list.c   | 157 +++---
 .../staging/usbip/userspace/src/usbip_network.h    |   1 -
 drivers/staging/usbip/userspace/src/usbip_unbind.c |  92 +---
 drivers/staging/usbip/userspace/src/usbipd.c       |  26 +-
 drivers/staging/usbip/userspace/src/utils.c        |  51 +-
 24 files changed, 1299 insertions(+), 844 deletions(-)
 create mode 100644 drivers/staging/usbip/userspace/libsrc/build_assert.h
 create mode 100644 drivers/staging/usbip/userspace/libsrc/check_type.h
 create mode 100644 drivers/staging/usbip/userspace/libsrc/container_of.h
 create mode 100644 drivers/staging/usbip/userspace/libsrc/list.c
 create mode 100644 drivers/staging/usbip/userspace/libsrc/list.h
 create mode 100644 drivers/staging/usbip/userspace/libsrc/sysfs_utils.c
 create mode 100644 drivers/staging/usbip/userspace/libsrc/sysfs_utils.h

-- 
1.8.1.2

[PATCH 01/12] staging: usbip: userspace: migrate usbip_bind to libudev

[Valentina Manea]

This patch adds autoconf check for libudev and migrates
usbip_bind to the new library.

libsysfs will still be used until all userspace is modified.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/configure.ac       |   6 +
 .../staging/usbip/userspace/libsrc/usbip_common.h  |   9 ++
 drivers/staging/usbip/userspace/src/Makefile.am    |   3 +-
 drivers/staging/usbip/userspace/src/sysfs_utils.c  |  36 +++++
 drivers/staging/usbip/userspace/src/sysfs_utils.h  |   8 ++
 drivers/staging/usbip/userspace/src/usbip_bind.c   | 149 +++++++++------------
 drivers/staging/usbip/userspace/src/utils.c        |  51 +++----
 7 files changed, 136 insertions(+), 126 deletions(-)
 create mode 100644 drivers/staging/usbip/userspace/src/sysfs_utils.c
 create mode 100644 drivers/staging/usbip/userspace/src/sysfs_utils.h

diff --git a/drivers/staging/usbip/userspace/configure.ac b/drivers/staging/usbip/userspace/configure.ac
index 0ee5d92..a5193c6 100644
--- a/drivers/staging/usbip/userspace/configure.ac
+++ b/drivers/staging/usbip/userspace/configure.ac
@@ -50,6 +50,12 @@ AC_CHECK_HEADER([sysfs/libsysfs.h],
 			      [AC_MSG_ERROR([Missing sysfs2 library!])])],
 		[AC_MSG_ERROR([Missing /usr/include/sysfs/libsysfs.h])])
 
+AC_CHECK_HEADER([libudev.h],
+		[AC_CHECK_LIB([udev], [udev_new],
+			      [LIBS="$LIBS -ludev"],
+			      [AC_MSG_ERROR([Missing udev library!])])],
+		[AC_MSG_ERROR([Missing /usr/include/libudev.h])])
+
 # Checks for libwrap library.
 AC_MSG_CHECKING([whether to use the libwrap (TCP wrappers) library])
 AC_ARG_WITH([tcp-wrappers],
diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
index 2cb81b3..565ac78 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
@@ -29,6 +29,15 @@
 #define USBIP_HOST_DRV_NAME	"usbip-host"
 #define USBIP_VHCI_DRV_NAME	"vhci_hcd"
 
+/* sysfs constants */
+#define SYSFS_MNT_PATH         "/sys"
+#define SYSFS_BUS_NAME         "bus"
+#define SYSFS_BUS_TYPE         "usb"
+#define SYSFS_DRIVERS_NAME     "drivers"
+
+#define SYSFS_PATH_MAX		256
+#define SYSFS_BUS_ID_SIZE	32
+
 extern int usbip_use_syslog;
 extern int usbip_use_stderr;
 extern int usbip_use_debug ;
diff --git a/drivers/staging/usbip/userspace/src/Makefile.am b/drivers/staging/usbip/userspace/src/Makefile.am
index b4f8c4b..6c91bcb 100644
--- a/drivers/staging/usbip/userspace/src/Makefile.am
+++ b/drivers/staging/usbip/userspace/src/Makefile.am
@@ -6,7 +6,8 @@ sbin_PROGRAMS := usbip usbipd
 
 usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
 		 usbip_attach.c usbip_detach.c usbip_list.c \
-		 usbip_bind.c usbip_unbind.c usbip_port.c
+		 usbip_bind.c usbip_unbind.c usbip_port.c \
+		 sysfs_utils.c
 
 
 usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
diff --git a/drivers/staging/usbip/userspace/src/sysfs_utils.c b/drivers/staging/usbip/userspace/src/sysfs_utils.c
new file mode 100644
index 0000000..2c362d1
--- /dev/null
+++ b/drivers/staging/usbip/userspace/src/sysfs_utils.c
@@ -0,0 +1,36 @@
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "sysfs_utils.h"
+#include "usbip_common.h"
+
+int write_sysfs_attribute(const char *attr_path, const char *new_value,
+			  size_t len)
+{
+	int fd;
+	int length;
+
+	if (attr_path == NULL || new_value == NULL || len == 0) {
+		dbg("Invalid values provided for attribute %s.", attr_path);
+		errno = EINVAL;
+		return -1;
+	}
+
+	if ((fd = open(attr_path, O_WRONLY)) < 0) {
+		dbg("Error opening attribute %s.", attr_path);
+		return -1;
+	}
+
+	length = write(fd, new_value, len);
+	if (length < 0) {
+		dbg("Error writing to attribute %s.", attr_path);
+		close(fd);
+		return -1;
+	}
+
+	close(fd);
+
+	return 0;
+}
diff --git a/drivers/staging/usbip/userspace/src/sysfs_utils.h b/drivers/staging/usbip/userspace/src/sysfs_utils.h
new file mode 100644
index 0000000..32ac1d1
--- /dev/null
+++ b/drivers/staging/usbip/userspace/src/sysfs_utils.h
@@ -0,0 +1,8 @@
+
+#ifndef __SYSFS_UTILS_H
+#define __SYSFS_UTILS_H
+
+int write_sysfs_attribute(const char *attr_path, const char *new_value,
+			  size_t len);
+
+#endif
diff --git a/drivers/staging/usbip/userspace/src/usbip_bind.c b/drivers/staging/usbip/userspace/src/usbip_bind.c
index 8cfd2db..d122089 100644
--- a/drivers/staging/usbip/userspace/src/usbip_bind.c
+++ b/drivers/staging/usbip/userspace/src/usbip_bind.c
@@ -16,7 +16,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <sysfs/libsysfs.h>
+#include <libudev.h>
 
 #include <errno.h>
 #include <stdio.h>
@@ -28,6 +28,7 @@
 #include "usbip_common.h"
 #include "utils.h"
 #include "usbip.h"
+#include "sysfs_utils.h"
 
 enum unbind_status {
 	UNBIND_ST_OK,
@@ -48,135 +49,94 @@ void usbip_bind_usage(void)
 /* call at unbound state */
 static int bind_usbip(char *busid)
 {
-	char bus_type[] = "usb";
 	char attr_name[] = "bind";
-	char sysfs_mntpath[SYSFS_PATH_MAX];
 	char bind_attr_path[SYSFS_PATH_MAX];
-	struct sysfs_attribute *bind_attr;
-	int failed = 0;
-	int rc, ret = -1;
-
-	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
-	if (rc < 0) {
-		err("sysfs must be mounted: %s", strerror(errno));
-		return -1;
-	}
+	int rc = -1;
 
 	snprintf(bind_attr_path, sizeof(bind_attr_path), "%s/%s/%s/%s/%s/%s",
-		 sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
-		 USBIP_HOST_DRV_NAME, attr_name);
+		 SYSFS_MNT_PATH, SYSFS_BUS_NAME, SYSFS_BUS_TYPE,
+		 SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME, attr_name);
+	dbg("bind attribute path: %s", bind_attr_path);
 
-	bind_attr = sysfs_open_attribute(bind_attr_path);
-	if (!bind_attr) {
-		dbg("problem getting bind attribute: %s", strerror(errno));
-		return -1;
-	}
-
-	rc = sysfs_write_attribute(bind_attr, busid, SYSFS_BUS_ID_SIZE);
+	rc = write_sysfs_attribute(bind_attr_path, busid, strlen(busid));
 	if (rc < 0) {
-		dbg("bind driver at %s failed", busid);
-		failed = 1;
+		dbg("Error binding device %s to driver: %s", busid,
+		    strerror(errno));
+		return -1;
 	}
 
-	if (!failed)
-		ret = 0;
-
-	sysfs_close_attribute(bind_attr);
-
-	return ret;
+	return 0;
 }
 
 /* buggy driver may cause dead lock */
 static int unbind_other(char *busid)
 {
-	char bus_type[] = "usb";
-	struct sysfs_device *busid_dev;
-	struct sysfs_device *dev;
-	struct sysfs_driver *drv;
-	struct sysfs_attribute *unbind_attr;
-	struct sysfs_attribute *bDevClass;
-	int rc;
 	enum unbind_status status = UNBIND_ST_OK;
 
-	busid_dev = sysfs_open_device(bus_type, busid);
-	if (!busid_dev) {
-		dbg("sysfs_open_device %s failed: %s", busid, strerror(errno));
-		return -1;
-	}
-	dbg("busid path: %s", busid_dev->path);
+	char attr_name[] = "unbind";
+	char unbind_attr_path[SYSFS_PATH_MAX];
+	int rc = -1;
 
-	bDevClass  = sysfs_get_device_attr(busid_dev, "bDeviceClass");
-	if (!bDevClass) {
-		dbg("problem getting device attribute: %s",
-		    strerror(errno));
+	struct udev *udev;
+	struct udev_device *dev;
+	const char *driver;
+	const char *bDevClass;
+
+	/* Create libudev context. */
+	udev = udev_new();
+
+	/* Get the device. */
+	dev = udev_device_new_from_subsystem_sysname(udev, "usb", busid);
+	if (!dev) {
+		dbg("Unable to find device with bus ID %s.", busid);
 		goto err_close_busid_dev;
 	}
 
-	if (!strncmp(bDevClass->value, "09", bDevClass->len)) {
-		dbg("skip unbinding of hub");
+	/* Check what kind of device it is. */
+	bDevClass  = udev_device_get_sysattr_value(dev, "bDeviceClass");
+	if (!bDevClass) {
+		dbg("Unable to get bDevClass device attribute.");
 		goto err_close_busid_dev;
 	}
 
-	dev = sysfs_open_device(bus_type, busid);
-	if (!dev) {
-		dbg("could not open device: %s",
-				strerror(errno));
+	if (!strncmp(bDevClass, "09", strlen(bDevClass))) {
+		dbg("Skip unbinding of hub.");
 		goto err_close_busid_dev;
 	}
 
-	dbg("%s -> %s", dev->name,  dev->driver_name);
-
-	if (!strncmp("unknown", dev->driver_name, SYSFS_NAME_LEN)) {
-		/* unbound interface */
-		sysfs_close_device(dev);
+	/* Get the device driver. */
+	driver = udev_device_get_driver(dev);
+	if (!driver) {
+		/* No driver bound to this device. */
 		goto out;
 	}
 
-	if (!strncmp(USBIP_HOST_DRV_NAME, dev->driver_name,
-				SYSFS_NAME_LEN)) {
-		/* already bound to usbip-host */
+	if (!strncmp(USBIP_HOST_DRV_NAME, driver,
+				strlen(USBIP_HOST_DRV_NAME))) {
+		/* Already bound to usbip-host. */
 		status = UNBIND_ST_USBIP_HOST;
-		sysfs_close_device(dev);
 		goto out;
 	}
 
-	/* unbinding */
-	drv = sysfs_open_driver(bus_type, dev->driver_name);
-	if (!drv) {
-		dbg("could not open device driver on %s: %s",
-				dev->name, strerror(errno));
-		goto err_close_intf_dev;
-	}
-	dbg("device driver: %s", drv->path);
-
-	unbind_attr = sysfs_get_driver_attr(drv, "unbind");
-	if (!unbind_attr) {
-		dbg("problem getting device driver attribute: %s",
-				strerror(errno));
-		goto err_close_intf_drv;
-	}
+	/* Unbind device from driver. */
+	snprintf(unbind_attr_path, sizeof(unbind_attr_path), "%s/%s/%s/%s/%s/%s",
+		 SYSFS_MNT_PATH, SYSFS_BUS_NAME, SYSFS_BUS_TYPE,
+		 SYSFS_DRIVERS_NAME, driver, attr_name);
+	dbg("unbind attribute path: %s", unbind_attr_path);
 
-	rc = sysfs_write_attribute(unbind_attr, dev->bus_id,
-			SYSFS_BUS_ID_SIZE);
+	rc = write_sysfs_attribute(unbind_attr_path, busid, strlen(busid));
 	if (rc < 0) {
-		/* NOTE: why keep unbinding other interfaces? */
-		dbg("unbind driver at %s failed", dev->bus_id);
-		status = UNBIND_ST_FAILED;
+		dbg("Error unbinding device %s from driver.", busid);
+		goto err_close_busid_dev;
 	}
 
-	sysfs_close_driver(drv);
-	sysfs_close_device(dev);
-
 	goto out;
 
-err_close_intf_drv:
-	sysfs_close_driver(drv);
-err_close_intf_dev:
-	sysfs_close_device(dev);
 err_close_busid_dev:
 	status = UNBIND_ST_FAILED;
 out:
-	sysfs_close_device(busid_dev);
+	udev_device_unref(dev);
+	udev_unref(udev);
 
 	return status;
 }
@@ -184,6 +144,17 @@ out:
 static int bind_device(char *busid)
 {
 	int rc;
+	struct udev *udev;
+	struct udev_device *dev;
+
+	/* Check whether the device with this bus ID exists. */
+	udev = udev_new();
+	dev = udev_device_new_from_subsystem_sysname(udev, "usb", busid);
+	if (!dev) {
+		err("Device with the specified bus ID does not exist.");
+		return -1;
+	}
+	udev_unref(udev);
 
 	rc = unbind_other(busid);
 	if (rc == UNBIND_ST_FAILED) {
diff --git a/drivers/staging/usbip/userspace/src/utils.c b/drivers/staging/usbip/userspace/src/utils.c
index 2d4966e..c75acac 100644
--- a/drivers/staging/usbip/userspace/src/utils.c
+++ b/drivers/staging/usbip/userspace/src/utils.c
@@ -16,61 +16,40 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <sysfs/libsysfs.h>
-
 #include <errno.h>
 #include <stdio.h>
 #include <string.h>
 
 #include "usbip_common.h"
 #include "utils.h"
+#include "sysfs_utils.h"
 
 int modify_match_busid(char *busid, int add)
 {
-	char bus_type[] = "usb";
 	char attr_name[] = "match_busid";
-	char buff[SYSFS_BUS_ID_SIZE + 4];
-	char sysfs_mntpath[SYSFS_PATH_MAX];
+	char command[SYSFS_BUS_ID_SIZE + 4];
 	char match_busid_attr_path[SYSFS_PATH_MAX];
-	struct sysfs_attribute *match_busid_attr;
-	int rc, ret = 0;
-
-	if (strnlen(busid, SYSFS_BUS_ID_SIZE) > SYSFS_BUS_ID_SIZE - 1) {
-		dbg("busid is too long");
-		return -1;
-	}
-
-	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
-	if (rc < 0) {
-		err("sysfs must be mounted: %s", strerror(errno));
-		return -1;
-	}
+	int rc;
 
 	snprintf(match_busid_attr_path, sizeof(match_busid_attr_path),
-		 "%s/%s/%s/%s/%s/%s", sysfs_mntpath, SYSFS_BUS_NAME, bus_type,
-		 SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME, attr_name);
-
-	match_busid_attr = sysfs_open_attribute(match_busid_attr_path);
-	if (!match_busid_attr) {
-		dbg("problem getting match_busid attribute: %s",
-		    strerror(errno));
-		return -1;
-	}
+		 "%s/%s/%s/%s/%s/%s", SYSFS_MNT_PATH, SYSFS_BUS_NAME,
+		 SYSFS_BUS_TYPE, SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME,
+		 attr_name);
+	dbg("match_busid attribute path: %s", match_busid_attr_path);
 
 	if (add)
-		snprintf(buff, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
+		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
 	else
-		snprintf(buff, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
+		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
 
-	dbg("write \"%s\" to %s", buff, match_busid_attr->path);
+	dbg("write \"%s\" to %s", command, match_busid_attr_path);
 
-	rc = sysfs_write_attribute(match_busid_attr, buff, sizeof(buff));
+	rc = write_sysfs_attribute(match_busid_attr_path, command,
+				   sizeof(command));
 	if (rc < 0) {
-		dbg("failed to write match_busid: %s", strerror(errno));
-		ret = -1;
+		dbg("Failed to write match_busid: %s", strerror(errno));
+		return -1;
 	}
 
-	sysfs_close_attribute(match_busid_attr);
-
-	return ret;
+	return 0;
 }
-- 
1.8.1.2

Re: [PATCH 01/12] staging: usbip: userspace: migrate usbip_bind to libudev

[Dan Carpenter]

On Tue, Mar 04, 2014 at 09:10:41PM +0200, Valentina Manea wrote:
> diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> index 2cb81b3..565ac78 100644
> --- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> +++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> @@ -29,6 +29,15 @@
>  #define USBIP_HOST_DRV_NAME	"usbip-host"
>  #define USBIP_VHCI_DRV_NAME	"vhci_hcd"
>  
> +/* sysfs constants */
> +#define SYSFS_MNT_PATH         "/sys"
> +#define SYSFS_BUS_NAME         "bus"
> +#define SYSFS_BUS_TYPE         "usb"
> +#define SYSFS_DRIVERS_NAME     "drivers"

What on earth???  I don't even want to review any further than this.

Groan....  No.  no no no no no.

regards,
dan carpenter

Re: [PATCH 01/12] staging: usbip: userspace: migrate usbip_bind to libudev

[Dan Carpenter]

On Wed, Mar 05, 2014 at 12:42:59PM +0300, Dan Carpenter wrote:
> On Tue, Mar 04, 2014 at 09:10:41PM +0200, Valentina Manea wrote:
> > diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> > index 2cb81b3..565ac78 100644
> > --- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> > +++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> > @@ -29,6 +29,15 @@
> >  #define USBIP_HOST_DRV_NAME	"usbip-host"
> >  #define USBIP_VHCI_DRV_NAME	"vhci_hcd"
> >  
> > +/* sysfs constants */
> > +#define SYSFS_MNT_PATH         "/sys"
> > +#define SYSFS_BUS_NAME         "bus"
> > +#define SYSFS_BUS_TYPE         "usb"
> > +#define SYSFS_DRIVERS_NAME     "drivers"
> 
> What on earth???  I don't even want to review any further than this.
> 
> Groan....  No.  no no no no no.
> 

I overreacted.  This stuff was already in the original code, and this
change just moves it around.

Fine fine.  These patch set seem ok with me.

regards,
dan carpenter

Re: [PATCH 01/12] staging: usbip: userspace: migrate usbip_bind to libudev

[Valentina Manea]

On Wed, Mar 5, 2014 at 12:15 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Wed, Mar 05, 2014 at 12:42:59PM +0300, Dan Carpenter wrote:
>> On Tue, Mar 04, 2014 at 09:10:41PM +0200, Valentina Manea wrote:
>> > diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
>> > index 2cb81b3..565ac78 100644
>> > --- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
>> > +++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
>> > @@ -29,6 +29,15 @@
>> >  #define USBIP_HOST_DRV_NAME        "usbip-host"
>> >  #define USBIP_VHCI_DRV_NAME        "vhci_hcd"
>> >
>> > +/* sysfs constants */
>> > +#define SYSFS_MNT_PATH         "/sys"
>> > +#define SYSFS_BUS_NAME         "bus"
>> > +#define SYSFS_BUS_TYPE         "usb"
>> > +#define SYSFS_DRIVERS_NAME     "drivers"
>>
>> What on earth???  I don't even want to review any further than this.
>>
>> Groan....  No.  no no no no no.
>>
>
> I overreacted.  This stuff was already in the original code, and this
> change just moves it around.
>
> Fine fine.  These patch set seem ok with me.
>
> regards,
> dan carpenter
>
>

Indeed, there are improvements than can be made to the code but
I figured doing them all in just one patch series would be a step too big.
This should be work for future patches.

Thanks for your feedback.

Valentina

Re: [PATCH 01/12] staging: usbip: userspace: migrate usbip_bind to libudev

[Dan Carpenter]

On Tue, Mar 04, 2014 at 09:10:41PM +0200, Valentina Manea wrote:

> +int write_sysfs_attribute(const char *attr_path, const char *new_value,
> +			  size_t len)
> +{
> +	int fd;
> +	int length;
> +
> +	if (attr_path == NULL || new_value == NULL || len == 0) {

Can any of these conditions be true?


> +		dbg("Invalid values provided for attribute %s.", attr_path);
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
> +	if ((fd = open(attr_path, O_WRONLY)) < 0) {

Move the assignment out of the condition.

> +		dbg("Error opening attribute %s.", attr_path);
> +		return -1;
> +	}
> +
> +	length = write(fd, new_value, len);
> +	if (length < 0) {
> +		dbg("Error writing to attribute %s.", attr_path);
> +		close(fd);
> +		return -1;
> +	}
> +
> +	close(fd);
> +
> +	return 0;
> +}
> diff --git a/drivers/staging/usbip/userspace/src/sysfs_utils.h b/drivers/staging/usbip/userspace/src/sysfs_utils.h
> new file mode 100644
> index 0000000..32ac1d1
> --- /dev/null
> +++ b/drivers/staging/usbip/userspace/src/sysfs_utils.h
> @@ -0,0 +1,8 @@
> +
> +#ifndef __SYSFS_UTILS_H
> +#define __SYSFS_UTILS_H
> +
> +int write_sysfs_attribute(const char *attr_path, const char *new_value,
> +			  size_t len);
> +
> +#endif
> diff --git a/drivers/staging/usbip/userspace/src/usbip_bind.c b/drivers/staging/usbip/userspace/src/usbip_bind.c
> index 8cfd2db..d122089 100644
> --- a/drivers/staging/usbip/userspace/src/usbip_bind.c
> +++ b/drivers/staging/usbip/userspace/src/usbip_bind.c
> @@ -16,7 +16,7 @@
>   * along with this program. If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#include <sysfs/libsysfs.h>
> +#include <libudev.h>
>  
>  #include <errno.h>
>  #include <stdio.h>
> @@ -28,6 +28,7 @@
>  #include "usbip_common.h"
>  #include "utils.h"
>  #include "usbip.h"
> +#include "sysfs_utils.h"
>  
>  enum unbind_status {
>  	UNBIND_ST_OK,
> @@ -48,135 +49,94 @@ void usbip_bind_usage(void)
>  /* call at unbound state */
>  static int bind_usbip(char *busid)
>  {
> -	char bus_type[] = "usb";

Ok.  This code was already really stupid before this patch but now it's
even more idiotic.  Just put these things things directly into the
string instead of abstracting the abastractions of the abstract
abstractions.  Sheesh.

>  	char attr_name[] = "bind";
> -	char sysfs_mntpath[SYSFS_PATH_MAX];
>  	char bind_attr_path[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *bind_attr;
> -	int failed = 0;
> -	int rc, ret = -1;
> -
> -	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
> -	if (rc < 0) {
> -		err("sysfs must be mounted: %s", strerror(errno));
> -		return -1;
> -	}
> +	int rc = -1;

Don't do pointless assignments.  The compiler has builtin checks for
uninitialized variables but now you are turning the checks off and the
compiler can't do its job.

>  
>  	snprintf(bind_attr_path, sizeof(bind_attr_path), "%s/%s/%s/%s/%s/%s",
> -		 sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
> -		 USBIP_HOST_DRV_NAME, attr_name);
> +		 SYSFS_MNT_PATH, SYSFS_BUS_NAME, SYSFS_BUS_TYPE,
> +		 SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME, attr_name);
> +	dbg("bind attribute path: %s", bind_attr_path);

These dbg statements are totally pointless.  Just use strace.

>  
> -	bind_attr = sysfs_open_attribute(bind_attr_path);
> -	if (!bind_attr) {
> -		dbg("problem getting bind attribute: %s", strerror(errno));
> -		return -1;
> -	}
> -
> -	rc = sysfs_write_attribute(bind_attr, busid, SYSFS_BUS_ID_SIZE);
> +	rc = write_sysfs_attribute(bind_attr_path, busid, strlen(busid));
>  	if (rc < 0) {
> -		dbg("bind driver at %s failed", busid);
> -		failed = 1;
> +		dbg("Error binding device %s to driver: %s", busid,
> +		    strerror(errno));

The temptation here would be to change the debug statement to an on by
default print statement.  The other option would be to just remove all
the debug statements.

Etc...  For the rest of this patch.  Remove abstraction levels and dead
code.

regards,
dan carpenter

Re: [PATCH 01/12] staging: usbip: userspace: migrate usbip_bind to libudev

[Shuah Khan]

On 03/04/2014 12:10 PM, Valentina Manea wrote:
> This patch adds autoconf check for libudev and migrates
> usbip_bind to the new library.
>
> libsysfs will still be used until all userspace is modified.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
> ---
>   drivers/staging/usbip/userspace/configure.ac       |   6 +
>   .../staging/usbip/userspace/libsrc/usbip_common.h  |   9 ++
>   drivers/staging/usbip/userspace/src/Makefile.am    |   3 +-
>   drivers/staging/usbip/userspace/src/sysfs_utils.c  |  36 +++++
>   drivers/staging/usbip/userspace/src/sysfs_utils.h  |   8 ++
>   drivers/staging/usbip/userspace/src/usbip_bind.c   | 149 +++++++++------------
>   drivers/staging/usbip/userspace/src/utils.c        |  51 +++----
>   7 files changed, 136 insertions(+), 126 deletions(-)
>   create mode 100644 drivers/staging/usbip/userspace/src/sysfs_utils.c
>   create mode 100644 drivers/staging/usbip/userspace/src/sysfs_utils.h
>
> diff --git a/drivers/staging/usbip/userspace/configure.ac b/drivers/staging/usbip/userspace/configure.ac
> index 0ee5d92..a5193c6 100644
> --- a/drivers/staging/usbip/userspace/configure.ac
> +++ b/drivers/staging/usbip/userspace/configure.ac
> @@ -50,6 +50,12 @@ AC_CHECK_HEADER([sysfs/libsysfs.h],
>   			      [AC_MSG_ERROR([Missing sysfs2 library!])])],
>   		[AC_MSG_ERROR([Missing /usr/include/sysfs/libsysfs.h])])
>
> +AC_CHECK_HEADER([libudev.h],
> +		[AC_CHECK_LIB([udev], [udev_new],
> +			      [LIBS="$LIBS -ludev"],
> +			      [AC_MSG_ERROR([Missing udev library!])])],
> +		[AC_MSG_ERROR([Missing /usr/include/libudev.h])])
> +
>   # Checks for libwrap library.
>   AC_MSG_CHECKING([whether to use the libwrap (TCP wrappers) library])
>   AC_ARG_WITH([tcp-wrappers],
> diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> index 2cb81b3..565ac78 100644
> --- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> +++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> @@ -29,6 +29,15 @@
>   #define USBIP_HOST_DRV_NAME	"usbip-host"
>   #define USBIP_VHCI_DRV_NAME	"vhci_hcd"
>
> +/* sysfs constants */
> +#define SYSFS_MNT_PATH         "/sys"
> +#define SYSFS_BUS_NAME         "bus"
> +#define SYSFS_BUS_TYPE         "usb"
> +#define SYSFS_DRIVERS_NAME     "drivers"
> +
> +#define SYSFS_PATH_MAX		256
> +#define SYSFS_BUS_ID_SIZE	32

I wish we have some global defines we could use.

> +
>   extern int usbip_use_syslog;
>   extern int usbip_use_stderr;
>   extern int usbip_use_debug ;
> diff --git a/drivers/staging/usbip/userspace/src/Makefile.am b/drivers/staging/usbip/userspace/src/Makefile.am
> index b4f8c4b..6c91bcb 100644
> --- a/drivers/staging/usbip/userspace/src/Makefile.am
> +++ b/drivers/staging/usbip/userspace/src/Makefile.am
> @@ -6,7 +6,8 @@ sbin_PROGRAMS := usbip usbipd
>
>   usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
>   		 usbip_attach.c usbip_detach.c usbip_list.c \
> -		 usbip_bind.c usbip_unbind.c usbip_port.c
> +		 usbip_bind.c usbip_unbind.c usbip_port.c \
> +		 sysfs_utils.c
>
>
>   usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
> diff --git a/drivers/staging/usbip/userspace/src/sysfs_utils.c b/drivers/staging/usbip/userspace/src/sysfs_utils.c
> new file mode 100644
> index 0000000..2c362d1
> --- /dev/null
> +++ b/drivers/staging/usbip/userspace/src/sysfs_utils.c
> @@ -0,0 +1,36 @@
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +
> +#include "sysfs_utils.h"
> +#include "usbip_common.h"
> +
> +int write_sysfs_attribute(const char *attr_path, const char *new_value,
> +			  size_t len)
> +{
> +	int fd;
> +	int length;
> +
> +	if (attr_path == NULL || new_value == NULL || len == 0) {
> +		dbg("Invalid values provided for attribute %s.", attr_path);
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
> +	if ((fd = open(attr_path, O_WRONLY)) < 0) {
> +		dbg("Error opening attribute %s.", attr_path);
> +		return -1;
> +	}
> +
> +	length = write(fd, new_value, len);
> +	if (length < 0) {
> +		dbg("Error writing to attribute %s.", attr_path);
> +		close(fd);
> +		return -1;
> +	}
> +
> +	close(fd);
> +
> +	return 0;
> +}
> diff --git a/drivers/staging/usbip/userspace/src/sysfs_utils.h b/drivers/staging/usbip/userspace/src/sysfs_utils.h
> new file mode 100644
> index 0000000..32ac1d1
> --- /dev/null
> +++ b/drivers/staging/usbip/userspace/src/sysfs_utils.h
> @@ -0,0 +1,8 @@
> +
> +#ifndef __SYSFS_UTILS_H
> +#define __SYSFS_UTILS_H
> +
> +int write_sysfs_attribute(const char *attr_path, const char *new_value,
> +			  size_t len);
> +
> +#endif
> diff --git a/drivers/staging/usbip/userspace/src/usbip_bind.c b/drivers/staging/usbip/userspace/src/usbip_bind.c
> index 8cfd2db..d122089 100644
> --- a/drivers/staging/usbip/userspace/src/usbip_bind.c
> +++ b/drivers/staging/usbip/userspace/src/usbip_bind.c
> @@ -16,7 +16,7 @@
>    * along with this program. If not, see <http://www.gnu.org/licenses/>.
>    */
>
> -#include <sysfs/libsysfs.h>
> +#include <libudev.h>
>
>   #include <errno.h>
>   #include <stdio.h>
> @@ -28,6 +28,7 @@
>   #include "usbip_common.h"
>   #include "utils.h"
>   #include "usbip.h"
> +#include "sysfs_utils.h"
>
>   enum unbind_status {
>   	UNBIND_ST_OK,
> @@ -48,135 +49,94 @@ void usbip_bind_usage(void)
>   /* call at unbound state */
>   static int bind_usbip(char *busid)
>   {
> -	char bus_type[] = "usb";
>   	char attr_name[] = "bind";
> -	char sysfs_mntpath[SYSFS_PATH_MAX];
>   	char bind_attr_path[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *bind_attr;
> -	int failed = 0;
> -	int rc, ret = -1;
> -
> -	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
> -	if (rc < 0) {
> -		err("sysfs must be mounted: %s", strerror(errno));
> -		return -1;
> -	}
> +	int rc = -1;
>
>   	snprintf(bind_attr_path, sizeof(bind_attr_path), "%s/%s/%s/%s/%s/%s",
> -		 sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
> -		 USBIP_HOST_DRV_NAME, attr_name);
> +		 SYSFS_MNT_PATH, SYSFS_BUS_NAME, SYSFS_BUS_TYPE,
> +		 SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME, attr_name);
> +	dbg("bind attribute path: %s", bind_attr_path);

Is this needed since there are error messages in the error paths?

>
> -	bind_attr = sysfs_open_attribute(bind_attr_path);
> -	if (!bind_attr) {
> -		dbg("problem getting bind attribute: %s", strerror(errno));
> -		return -1;
> -	}
> -
> -	rc = sysfs_write_attribute(bind_attr, busid, SYSFS_BUS_ID_SIZE);
> +	rc = write_sysfs_attribute(bind_attr_path, busid, strlen(busid));
>   	if (rc < 0) {
> -		dbg("bind driver at %s failed", busid);
> -		failed = 1;
> +		dbg("Error binding device %s to driver: %s", busid,
> +		    strerror(errno));

I think it would make sense to have this as an error as opposed to debug 
only message.

> +		return -1;
>   	}
>
> -	if (!failed)
> -		ret = 0;
> -
> -	sysfs_close_attribute(bind_attr);
> -
> -	return ret;
> +	return 0;
>   }
>
>   /* buggy driver may cause dead lock */
>   static int unbind_other(char *busid)
>   {
> -	char bus_type[] = "usb";
> -	struct sysfs_device *busid_dev;
> -	struct sysfs_device *dev;
> -	struct sysfs_driver *drv;
> -	struct sysfs_attribute *unbind_attr;
> -	struct sysfs_attribute *bDevClass;
> -	int rc;
>   	enum unbind_status status = UNBIND_ST_OK;
>
> -	busid_dev = sysfs_open_device(bus_type, busid);
> -	if (!busid_dev) {
> -		dbg("sysfs_open_device %s failed: %s", busid, strerror(errno));
> -		return -1;
> -	}
> -	dbg("busid path: %s", busid_dev->path);
> +	char attr_name[] = "unbind";
> +	char unbind_attr_path[SYSFS_PATH_MAX];
> +	int rc = -1;
>
> -	bDevClass  = sysfs_get_device_attr(busid_dev, "bDeviceClass");
> -	if (!bDevClass) {
> -		dbg("problem getting device attribute: %s",
> -		    strerror(errno));
> +	struct udev *udev;
> +	struct udev_device *dev;
> +	const char *driver;
> +	const char *bDevClass;
> +
> +	/* Create libudev context. */
> +	udev = udev_new();
> +
> +	/* Get the device. */
> +	dev = udev_device_new_from_subsystem_sysname(udev, "usb", busid);
> +	if (!dev) {
> +		dbg("Unable to find device with bus ID %s.", busid);
>   		goto err_close_busid_dev;
>   	}
>
> -	if (!strncmp(bDevClass->value, "09", bDevClass->len)) {
> -		dbg("skip unbinding of hub");
> +	/* Check what kind of device it is. */
> +	bDevClass  = udev_device_get_sysattr_value(dev, "bDeviceClass");
> +	if (!bDevClass) {
> +		dbg("Unable to get bDevClass device attribute.");
>   		goto err_close_busid_dev;
>   	}
>
> -	dev = sysfs_open_device(bus_type, busid);
> -	if (!dev) {
> -		dbg("could not open device: %s",
> -				strerror(errno));
> +	if (!strncmp(bDevClass, "09", strlen(bDevClass))) {
> +		dbg("Skip unbinding of hub.");
>   		goto err_close_busid_dev;
>   	}
>
> -	dbg("%s -> %s", dev->name,  dev->driver_name);
> -
> -	if (!strncmp("unknown", dev->driver_name, SYSFS_NAME_LEN)) {
> -		/* unbound interface */
> -		sysfs_close_device(dev);
> +	/* Get the device driver. */
> +	driver = udev_device_get_driver(dev);
> +	if (!driver) {
> +		/* No driver bound to this device. */
>   		goto out;
>   	}
>
> -	if (!strncmp(USBIP_HOST_DRV_NAME, dev->driver_name,
> -				SYSFS_NAME_LEN)) {
> -		/* already bound to usbip-host */
> +	if (!strncmp(USBIP_HOST_DRV_NAME, driver,
> +				strlen(USBIP_HOST_DRV_NAME))) {
> +		/* Already bound to usbip-host. */
>   		status = UNBIND_ST_USBIP_HOST;
> -		sysfs_close_device(dev);
>   		goto out;
>   	}
>
> -	/* unbinding */
> -	drv = sysfs_open_driver(bus_type, dev->driver_name);
> -	if (!drv) {
> -		dbg("could not open device driver on %s: %s",
> -				dev->name, strerror(errno));
> -		goto err_close_intf_dev;
> -	}
> -	dbg("device driver: %s", drv->path);
> -
> -	unbind_attr = sysfs_get_driver_attr(drv, "unbind");
> -	if (!unbind_attr) {
> -		dbg("problem getting device driver attribute: %s",
> -				strerror(errno));
> -		goto err_close_intf_drv;
> -	}
> +	/* Unbind device from driver. */
> +	snprintf(unbind_attr_path, sizeof(unbind_attr_path), "%s/%s/%s/%s/%s/%s",
> +		 SYSFS_MNT_PATH, SYSFS_BUS_NAME, SYSFS_BUS_TYPE,
> +		 SYSFS_DRIVERS_NAME, driver, attr_name);
> +	dbg("unbind attribute path: %s", unbind_attr_path);

I think this could be removed since there are error messages in failure 
legs. I am finding usbip userspace is rather prolific with debug. So it 
would help if err() is used in error legs as opposed to dbg() and use 
dbg() only for debug messages.

>
> -	rc = sysfs_write_attribute(unbind_attr, dev->bus_id,
> -			SYSFS_BUS_ID_SIZE);
> +	rc = write_sysfs_attribute(unbind_attr_path, busid, strlen(busid));
>   	if (rc < 0) {
> -		/* NOTE: why keep unbinding other interfaces? */
> -		dbg("unbind driver at %s failed", dev->bus_id);
> -		status = UNBIND_ST_FAILED;
> +		dbg("Error unbinding device %s from driver.", busid);

This could be err() instead of dbg()

> +		goto err_close_busid_dev;
>   	}
>
> -	sysfs_close_driver(drv);
> -	sysfs_close_device(dev);
> -
>   	goto out;
>
> -err_close_intf_drv:
> -	sysfs_close_driver(drv);
> -err_close_intf_dev:
> -	sysfs_close_device(dev);
>   err_close_busid_dev:
>   	status = UNBIND_ST_FAILED;
>   out:
> -	sysfs_close_device(busid_dev);
> +	udev_device_unref(dev);
> +	udev_unref(udev);
>
>   	return status;
>   }
> @@ -184,6 +144,17 @@ out:
>   static int bind_device(char *busid)
>   {
>   	int rc;
> +	struct udev *udev;
> +	struct udev_device *dev;
> +
> +	/* Check whether the device with this bus ID exists. */
> +	udev = udev_new();
> +	dev = udev_device_new_from_subsystem_sysname(udev, "usb", busid);
> +	if (!dev) {
> +		err("Device with the specified bus ID does not exist.");
> +		return -1;
> +	}
> +	udev_unref(udev);
>
>   	rc = unbind_other(busid);
>   	if (rc == UNBIND_ST_FAILED) {
> diff --git a/drivers/staging/usbip/userspace/src/utils.c b/drivers/staging/usbip/userspace/src/utils.c
> index 2d4966e..c75acac 100644
> --- a/drivers/staging/usbip/userspace/src/utils.c
> +++ b/drivers/staging/usbip/userspace/src/utils.c
> @@ -16,61 +16,40 @@
>    * along with this program. If not, see <http://www.gnu.org/licenses/>.
>    */
>
> -#include <sysfs/libsysfs.h>
> -
>   #include <errno.h>
>   #include <stdio.h>
>   #include <string.h>
>
>   #include "usbip_common.h"
>   #include "utils.h"
> +#include "sysfs_utils.h"
>
>   int modify_match_busid(char *busid, int add)
>   {
> -	char bus_type[] = "usb";
>   	char attr_name[] = "match_busid";
> -	char buff[SYSFS_BUS_ID_SIZE + 4];
> -	char sysfs_mntpath[SYSFS_PATH_MAX];
> +	char command[SYSFS_BUS_ID_SIZE + 4];
>   	char match_busid_attr_path[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *match_busid_attr;
> -	int rc, ret = 0;
> -
> -	if (strnlen(busid, SYSFS_BUS_ID_SIZE) > SYSFS_BUS_ID_SIZE - 1) {
> -		dbg("busid is too long");
> -		return -1;
> -	}
> -
> -	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
> -	if (rc < 0) {
> -		err("sysfs must be mounted: %s", strerror(errno));
> -		return -1;
> -	}
> +	int rc;
>
>   	snprintf(match_busid_attr_path, sizeof(match_busid_attr_path),
> -		 "%s/%s/%s/%s/%s/%s", sysfs_mntpath, SYSFS_BUS_NAME, bus_type,
> -		 SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME, attr_name);
> -
> -	match_busid_attr = sysfs_open_attribute(match_busid_attr_path);
> -	if (!match_busid_attr) {
> -		dbg("problem getting match_busid attribute: %s",
> -		    strerror(errno));
> -		return -1;
> -	}
> +		 "%s/%s/%s/%s/%s/%s", SYSFS_MNT_PATH, SYSFS_BUS_NAME,
> +		 SYSFS_BUS_TYPE, SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME,
> +		 attr_name);
> +	dbg("match_busid attribute path: %s", match_busid_attr_path);
>
>   	if (add)
> -		snprintf(buff, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
> +		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
>   	else
> -		snprintf(buff, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
> +		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
>
> -	dbg("write \"%s\" to %s", buff, match_busid_attr->path);
> +	dbg("write \"%s\" to %s", command, match_busid_attr_path);

This messge could be removed.

>
> -	rc = sysfs_write_attribute(match_busid_attr, buff, sizeof(buff));
> +	rc = write_sysfs_attribute(match_busid_attr_path, command,
> +				   sizeof(command));
>   	if (rc < 0) {
> -		dbg("failed to write match_busid: %s", strerror(errno));
> -		ret = -1;
> +		dbg("Failed to write match_busid: %s", strerror(errno));
> +		return -1;
>   	}
>
> -	sysfs_close_attribute(match_busid_attr);
> -
> -	return ret;
> +	return 0;
>   }
>

Good work. Thanks for doing this. You have my Reviewed-by after making 
the recommended changes.

Reviewed-by: Shuah Khan <shuah.kh@samsung.com>

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

Re: [PATCH 01/12] staging: usbip: userspace: migrate usbip_bind to libudev

[Dan Carpenter]

On Thu, Mar 06, 2014 at 09:15:01AM -0700, Shuah Khan wrote:
> >--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> >+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> >@@ -29,6 +29,15 @@
> >  #define USBIP_HOST_DRV_NAME	"usbip-host"
> >  #define USBIP_VHCI_DRV_NAME	"vhci_hcd"
> >
> >+/* sysfs constants */
> >+#define SYSFS_MNT_PATH         "/sys"
> >+#define SYSFS_BUS_NAME         "bus"
> >+#define SYSFS_BUS_TYPE         "usb"
> >+#define SYSFS_DRIVERS_NAME     "drivers"
> >+
> >+#define SYSFS_PATH_MAX		256
> >+#define SYSFS_BUS_ID_SIZE	32
> 
> I wish we have some global defines we could use.

Except for the mount point, the rest of the path is part of the kernel
API.

regards,
dan carpenter

[PATCH 02/12] staging: usbip: userspace: remove useless libsysfs includes

[Valentina Manea]

This patch removes useless libsysfs.h includes in various
userspace files.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/src/usbip_attach.c  | 1 -
 drivers/staging/usbip/userspace/src/usbip_detach.c  | 2 --
 drivers/staging/usbip/userspace/src/usbip_network.h | 1 -
 3 files changed, 4 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_attach.c b/drivers/staging/usbip/userspace/src/usbip_attach.c
index 0858411..716a79e 100644
--- a/drivers/staging/usbip/userspace/src/usbip_attach.c
+++ b/drivers/staging/usbip/userspace/src/usbip_attach.c
@@ -17,7 +17,6 @@
  */
 
 #include <sys/stat.h>
-#include <sysfs/libsysfs.h>
 
 #include <limits.h>
 #include <stdint.h>
diff --git a/drivers/staging/usbip/userspace/src/usbip_detach.c b/drivers/staging/usbip/userspace/src/usbip_detach.c
index 13308df..05c6d15 100644
--- a/drivers/staging/usbip/userspace/src/usbip_detach.c
+++ b/drivers/staging/usbip/userspace/src/usbip_detach.c
@@ -16,8 +16,6 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <sysfs/libsysfs.h>
-
 #include <ctype.h>
 #include <limits.h>
 #include <stdint.h>
diff --git a/drivers/staging/usbip/userspace/src/usbip_network.h b/drivers/staging/usbip/userspace/src/usbip_network.h
index f19ae19..c1e875c 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.h
+++ b/drivers/staging/usbip/userspace/src/usbip_network.h
@@ -10,7 +10,6 @@
 #endif
 
 #include <sys/types.h>
-#include <sysfs/libsysfs.h>
 
 #include <stdint.h>
 
-- 
1.8.1.2

[PATCH 03/12] staging: usbip: userspace: migrate usbip_unbind to libudev

[Valentina Manea]

This patch modifies usbip_unbind to use libudev.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/src/usbip_unbind.c | 92 +++++++---------------
 1 file changed, 29 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_unbind.c b/drivers/staging/usbip/userspace/src/usbip_unbind.c
index cace878..4776068 100644
--- a/drivers/staging/usbip/userspace/src/usbip_unbind.c
+++ b/drivers/staging/usbip/userspace/src/usbip_unbind.c
@@ -16,7 +16,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <sysfs/libsysfs.h>
+#include <libudev.h>
 
 #include <errno.h>
 #include <stdio.h>
@@ -27,6 +27,7 @@
 #include "usbip_common.h"
 #include "utils.h"
 #include "usbip.h"
+#include "sysfs_utils.h"
 
 static const char usbip_unbind_usage_string[] =
 	"usbip unbind <args>\n"
@@ -41,92 +42,57 @@ void usbip_unbind_usage(void)
 static int unbind_device(char *busid)
 {
 	char bus_type[] = "usb";
-	struct sysfs_driver *usbip_host_drv;
-	struct sysfs_device *dev;
-	struct dlist *devlist;
-	int verified = 0;
 	int rc, ret = -1;
 
 	char attr_name[] = "unbind";
-	char sysfs_mntpath[SYSFS_PATH_MAX];
 	char unbind_attr_path[SYSFS_PATH_MAX];
-	struct sysfs_attribute *unbind_attr;
-
-	/* verify the busid device is using usbip-host */
-	usbip_host_drv = sysfs_open_driver(bus_type, USBIP_HOST_DRV_NAME);
-	if (!usbip_host_drv) {
-		err("could not open %s driver: %s", USBIP_HOST_DRV_NAME,
-		    strerror(errno));
-		return -1;
-	}
 
-	devlist = sysfs_get_driver_devices(usbip_host_drv);
-	if (!devlist) {
-		err("%s is not in use by any devices", USBIP_HOST_DRV_NAME);
-		goto err_close_usbip_host_drv;
-	}
+	struct udev *udev;
+	struct udev_device *dev;
+	const char *driver;
 
-	dlist_for_each_data(devlist, dev, struct sysfs_device) {
-		if (!strncmp(busid, dev->name, strlen(busid)) &&
-		    !strncmp(dev->driver_name, USBIP_HOST_DRV_NAME,
-			     strlen(USBIP_HOST_DRV_NAME))) {
-			verified = 1;
-			break;
-		}
-	}
+	/* Create libudev context. */
+	udev = udev_new();
 
-	if (!verified) {
-		err("device on busid %s is not using %s", busid,
-		    USBIP_HOST_DRV_NAME);
-		goto err_close_usbip_host_drv;
+	/* Check whether the device with this bus ID exists. */
+	dev = udev_device_new_from_subsystem_sysname(udev, "usb", busid);
+	if (!dev) {
+		err("Device with the specified bus ID does not exist.");
+		goto err_close_udev;
 	}
 
-	/*
-	 * NOTE: A read and write of an attribute value of the device busid
-	 * refers to must be done to start probing. That way a rebind of the
-	 * default driver for the device occurs.
-	 *
-	 * This seems very hackish and adds a lot of pointless code. I think it
-	 * should be done in the kernel by the driver after del_match_busid is
-	 * finished!
-	 */
-
-	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
-	if (rc < 0) {
-		err("sysfs must be mounted: %s", strerror(errno));
-		return -1;
+	/* Check whether the device is using usbip-host driver. */
+	driver = udev_device_get_driver(dev);
+	if (!driver || strcmp(driver, "usbip-host")) {
+		err("Device is not bound to usbip-host driver.");
+		goto err_close_udev;
 	}
 
+	/* Unbind device from driver. */
 	snprintf(unbind_attr_path, sizeof(unbind_attr_path), "%s/%s/%s/%s/%s/%s",
-		 sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
+		 SYSFS_MNT_PATH, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
 		 USBIP_HOST_DRV_NAME, attr_name);
+	dbg("unbind attribute path: %s", unbind_attr_path);
 
-	/* read a device attribute */
-	unbind_attr = sysfs_open_attribute(unbind_attr_path);
-	if (!unbind_attr) {
-		err("could not open %s/%s: %s", busid, attr_name,
-		    strerror(errno));
-		return -1;
+	rc = write_sysfs_attribute(unbind_attr_path, busid, strlen(busid));
+	if (rc < 0) {
+		dbg("Error unbinding device %s from driver.", busid);
+		goto err_close_udev;
 	}
 
-	/* notify driver of unbind */
+	/* Notify driver of unbind. */
 	rc = modify_match_busid(busid, 0);
 	if (rc < 0) {
 		err("unable to unbind device on %s", busid);
+		goto err_close_udev;
 	}
 
-	rc = sysfs_write_attribute(unbind_attr, busid,
-				   SYSFS_BUS_ID_SIZE);
-		if (rc < 0) {
-			dbg("bind driver at %s failed", busid);
-		}
-	sysfs_close_attribute(unbind_attr);
-
 	ret = 0;
 	printf("unbind device on busid %s: complete\n", busid);
 
-err_close_usbip_host_drv:
-	sysfs_close_driver(usbip_host_drv);
+err_close_udev:
+	udev_device_unref(dev);
+	udev_unref(udev);
 
 	return ret;
 }
-- 
1.8.1.2

Re: [PATCH 03/12] staging: usbip: userspace: migrate usbip_unbind to libudev

[Shuah Khan]

On 03/04/2014 12:10 PM, Valentina Manea wrote:
> This patch modifies usbip_unbind to use libudev.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
> ---
>   drivers/staging/usbip/userspace/src/usbip_unbind.c | 92 +++++++---------------
>   1 file changed, 29 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/staging/usbip/userspace/src/usbip_unbind.c b/drivers/staging/usbip/userspace/src/usbip_unbind.c
> index cace878..4776068 100644
> --- a/drivers/staging/usbip/userspace/src/usbip_unbind.c
> +++ b/drivers/staging/usbip/userspace/src/usbip_unbind.c
> @@ -16,7 +16,7 @@
>    * along with this program. If not, see <http://www.gnu.org/licenses/>.
>    */
>
> -#include <sysfs/libsysfs.h>
> +#include <libudev.h>
>
>   #include <errno.h>
>   #include <stdio.h>
> @@ -27,6 +27,7 @@
>   #include "usbip_common.h"
>   #include "utils.h"
>   #include "usbip.h"
> +#include "sysfs_utils.h"
>
>   static const char usbip_unbind_usage_string[] =
>   	"usbip unbind <args>\n"
> @@ -41,92 +42,57 @@ void usbip_unbind_usage(void)
>   static int unbind_device(char *busid)
>   {
>   	char bus_type[] = "usb";
> -	struct sysfs_driver *usbip_host_drv;
> -	struct sysfs_device *dev;
> -	struct dlist *devlist;
> -	int verified = 0;
>   	int rc, ret = -1;
>
>   	char attr_name[] = "unbind";
> -	char sysfs_mntpath[SYSFS_PATH_MAX];
>   	char unbind_attr_path[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *unbind_attr;
> -
> -	/* verify the busid device is using usbip-host */
> -	usbip_host_drv = sysfs_open_driver(bus_type, USBIP_HOST_DRV_NAME);
> -	if (!usbip_host_drv) {
> -		err("could not open %s driver: %s", USBIP_HOST_DRV_NAME,
> -		    strerror(errno));
> -		return -1;
> -	}
>
> -	devlist = sysfs_get_driver_devices(usbip_host_drv);
> -	if (!devlist) {
> -		err("%s is not in use by any devices", USBIP_HOST_DRV_NAME);
> -		goto err_close_usbip_host_drv;
> -	}
> +	struct udev *udev;
> +	struct udev_device *dev;
> +	const char *driver;
>
> -	dlist_for_each_data(devlist, dev, struct sysfs_device) {
> -		if (!strncmp(busid, dev->name, strlen(busid)) &&
> -		    !strncmp(dev->driver_name, USBIP_HOST_DRV_NAME,
> -			     strlen(USBIP_HOST_DRV_NAME))) {
> -			verified = 1;
> -			break;
> -		}
> -	}
> +	/* Create libudev context. */
> +	udev = udev_new();
>
> -	if (!verified) {
> -		err("device on busid %s is not using %s", busid,
> -		    USBIP_HOST_DRV_NAME);
> -		goto err_close_usbip_host_drv;
> +	/* Check whether the device with this bus ID exists. */
> +	dev = udev_device_new_from_subsystem_sysname(udev, "usb", busid);
> +	if (!dev) {
> +		err("Device with the specified bus ID does not exist.");
> +		goto err_close_udev;
>   	}
>
> -	/*
> -	 * NOTE: A read and write of an attribute value of the device busid
> -	 * refers to must be done to start probing. That way a rebind of the
> -	 * default driver for the device occurs.
> -	 *
> -	 * This seems very hackish and adds a lot of pointless code. I think it
> -	 * should be done in the kernel by the driver after del_match_busid is
> -	 * finished!
> -	 */
> -
> -	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
> -	if (rc < 0) {
> -		err("sysfs must be mounted: %s", strerror(errno));
> -		return -1;
> +	/* Check whether the device is using usbip-host driver. */
> +	driver = udev_device_get_driver(dev);
> +	if (!driver || strcmp(driver, "usbip-host")) {
> +		err("Device is not bound to usbip-host driver.");
> +		goto err_close_udev;
>   	}
>
> +	/* Unbind device from driver. */
>   	snprintf(unbind_attr_path, sizeof(unbind_attr_path), "%s/%s/%s/%s/%s/%s",
> -		 sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
> +		 SYSFS_MNT_PATH, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
>   		 USBIP_HOST_DRV_NAME, attr_name);
> +	dbg("unbind attribute path: %s", unbind_attr_path);

Could you please remove this debug message.

>
> -	/* read a device attribute */
> -	unbind_attr = sysfs_open_attribute(unbind_attr_path);
> -	if (!unbind_attr) {
> -		err("could not open %s/%s: %s", busid, attr_name,
> -		    strerror(errno));
> -		return -1;
> +	rc = write_sysfs_attribute(unbind_attr_path, busid, strlen(busid));
> +	if (rc < 0) {
> +		dbg("Error unbinding device %s from driver.", busid);

Could you please make this an err()

> +		goto err_close_udev;
>   	}
>
> -	/* notify driver of unbind */
> +	/* Notify driver of unbind. */
>   	rc = modify_match_busid(busid, 0);
>   	if (rc < 0) {
>   		err("unable to unbind device on %s", busid);
> +		goto err_close_udev;
>   	}
>
> -	rc = sysfs_write_attribute(unbind_attr, busid,
> -				   SYSFS_BUS_ID_SIZE);
> -		if (rc < 0) {
> -			dbg("bind driver at %s failed", busid);
> -		}
> -	sysfs_close_attribute(unbind_attr);
> -
>   	ret = 0;
>   	printf("unbind device on busid %s: complete\n", busid);

Could you please change this to an info()

>
> -err_close_usbip_host_drv:
> -	sysfs_close_driver(usbip_host_drv);
> +err_close_udev:
> +	udev_device_unref(dev);
> +	udev_unref(udev);
>
>   	return ret;
>   }
>

You have my Reviewed-by after making the recommended changes.

Reviewed-by: Shuah Khan <shuah.kh@samsung.com>

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

[PATCH 04/12] staging: usbip: userspace: migrate usbip_list to libudev

[Valentina Manea]

This patch modifies usbip_list to use libudev.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/src/usbip_list.c | 137 +++++++++--------------
 1 file changed, 51 insertions(+), 86 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_list.c b/drivers/staging/usbip/userspace/src/usbip_list.c
index 8864fa2..2255107 100644
--- a/drivers/staging/usbip/userspace/src/usbip_list.c
+++ b/drivers/staging/usbip/userspace/src/usbip_list.c
@@ -17,7 +17,7 @@
  */
 
 #include <sys/types.h>
-#include <sysfs/libsysfs.h>
+#include <libudev.h>
 
 #include <errno.h>
 #include <stdbool.h>
@@ -133,8 +133,8 @@ static int list_exported_devices(char *host)
 	return 0;
 }
 
-static void print_device(char *busid, char *vendor, char *product,
-			 bool parsable)
+static void print_device(const char *busid, const char *vendor,
+			 const char *product, bool parsable)
 {
 	if (parsable)
 		printf("busid=%s#usbid=%.4s:%.4s#", busid, vendor, product);
@@ -148,106 +148,71 @@ static void print_product_name(char *product_name, bool parsable)
 		printf("   %s\n", product_name);
 }
 
-static void print_interface(char *busid, char *driver, bool parsable)
-{
-	if (parsable)
-		printf("%s=%s#", busid, driver);
-	else
-		printf("%9s%s -> %s\n", "", busid, driver);
-}
-
-static int is_device(void *x)
-{
-	struct sysfs_attribute *devpath;
-	struct sysfs_device *dev = x;
-	int ret = 0;
-
-	devpath = sysfs_get_device_attr(dev, "devpath");
-	if (devpath && *devpath->value != '0')
-		ret = 1;
-
-	return ret;
-}
-
-static int devcmp(void *a, void *b)
-{
-	return strcmp(a, b);
-}
-
 static int list_devices(bool parsable)
 {
-	char bus_type[] = "usb";
-	char busid[SYSFS_BUS_ID_SIZE];
+	struct udev *udev;
+	struct udev_enumerate *enumerate;
+	struct udev_list_entry *devices, *dev_list_entry;
+	struct udev_device *dev;
+	const char *path;
+	const char *idVendor;
+	const char *idProduct;
+	const char *bConfValue;
+	const char *bNumIntfs;
+	const char *busid;
 	char product_name[128];
-	struct sysfs_bus *ubus;
-	struct sysfs_device *dev;
-	struct sysfs_device *intf;
-	struct sysfs_attribute *idVendor;
-	struct sysfs_attribute *idProduct;
-	struct sysfs_attribute *bConfValue;
-	struct sysfs_attribute *bNumIntfs;
-	struct dlist *devlist;
-	int i;
 	int ret = -1;
 
-	ubus = sysfs_open_bus(bus_type);
-	if (!ubus) {
-		err("could not open %s bus: %s", bus_type, strerror(errno));
-		return -1;
-	}
-
-	devlist = sysfs_get_bus_devices(ubus);
-	if (!devlist) {
-		err("could not get %s bus devices: %s", bus_type,
-		    strerror(errno));
-		goto err_out;
-	}
-
-	/* remove interfaces and root hubs from device list */
-	dlist_filter_sort(devlist, is_device, devcmp);
-
-	if (!parsable) {
-		printf("Local USB devices\n");
-		printf("=================\n");
-	}
-	dlist_for_each_data(devlist, dev, struct sysfs_device) {
-		idVendor   = sysfs_get_device_attr(dev, "idVendor");
-		idProduct  = sysfs_get_device_attr(dev, "idProduct");
-		bConfValue = sysfs_get_device_attr(dev, "bConfigurationValue");
-		bNumIntfs  = sysfs_get_device_attr(dev, "bNumInterfaces");
+	/* Create libudev context. */
+	udev = udev_new();
+
+	/* Create libudev device enumeration. */
+	enumerate = udev_enumerate_new(udev);
+
+	/* Take only USB devices that are not hubs and do not have
+	 * the bInterfaceNumber attribute, i.e. are not interfaces.
+	 */
+	udev_enumerate_add_match_subsystem(enumerate, "usb");
+	udev_enumerate_add_nomatch_sysattr(enumerate, "bDeviceClass", "09");
+	udev_enumerate_add_nomatch_sysattr(enumerate, "bInterfaceNumber", NULL);
+	udev_enumerate_scan_devices(enumerate);
+
+	devices = udev_enumerate_get_list_entry(enumerate);
+
+	/* Show information about each device. */
+	udev_list_entry_foreach(dev_list_entry, devices) {
+		path = udev_list_entry_get_name(dev_list_entry);
+		dev = udev_device_new_from_syspath(udev, path);
+
+		/* Get device information. */
+		idVendor = udev_device_get_sysattr_value(dev, "idVendor");
+		idProduct = udev_device_get_sysattr_value(dev, "idProduct");
+		bConfValue = udev_device_get_sysattr_value(dev, "bConfigurationValue");
+		bNumIntfs = udev_device_get_sysattr_value(dev, "bNumInterfaces");
+		busid = udev_device_get_sysname(dev);
 		if (!idVendor || !idProduct || !bConfValue || !bNumIntfs) {
-			err("problem getting device attributes: %s",
+			err("Problem getting device attributes: %s",
 			    strerror(errno));
 			goto err_out;
 		}
 
-		/* get product name */
+		/* Get product name. */
 		usbip_names_get_product(product_name, sizeof(product_name),
-					strtol(idVendor->value, NULL, 16),
-					strtol(idProduct->value, NULL, 16));
-		print_device(dev->bus_id, idVendor->value, idProduct->value,
-			     parsable);
+					strtol(idVendor, NULL, 16),
+					strtol(idProduct, NULL, 16));
+
+		/* Print information. */
+		print_device(busid, idVendor, idProduct, parsable);
 		print_product_name(product_name, parsable);
 
-		for (i = 0; i < atoi(bNumIntfs->value); i++) {
-			snprintf(busid, sizeof(busid), "%s:%.1s.%d",
-				 dev->bus_id, bConfValue->value, i);
-			intf = sysfs_open_device(bus_type, busid);
-			if (!intf) {
-				err("could not open device interface: %s",
-				    strerror(errno));
-				goto err_out;
-			}
-			print_interface(busid, intf->driver_name, parsable);
-			sysfs_close_device(intf);
-		}
 		printf("\n");
-	}
 
-	ret = 0;
+		udev_device_unref(dev);
+	}
 
 err_out:
-	sysfs_close_bus(ubus);
+	udev_enumerate_unref(enumerate);
+	udev_unref(udev);
 
 	return ret;
 }
-- 
1.8.1.2

Re: [PATCH 04/12] staging: usbip: userspace: migrate usbip_list to libudev

[Shuah Khan]

On 03/04/2014 12:10 PM, Valentina Manea wrote:
> This patch modifies usbip_list to use libudev.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
> ---
>   drivers/staging/usbip/userspace/src/usbip_list.c | 137 +++++++++--------------
>   1 file changed, 51 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/staging/usbip/userspace/src/usbip_list.c b/drivers/staging/usbip/userspace/src/usbip_list.c
> index 8864fa2..2255107 100644
> --- a/drivers/staging/usbip/userspace/src/usbip_list.c
> +++ b/drivers/staging/usbip/userspace/src/usbip_list.c
> @@ -17,7 +17,7 @@
>    */
>
>   #include <sys/types.h>
> -#include <sysfs/libsysfs.h>
> +#include <libudev.h>
>
>   #include <errno.h>
>   #include <stdbool.h>
> @@ -133,8 +133,8 @@ static int list_exported_devices(char *host)
>   	return 0;
>   }
>
> -static void print_device(char *busid, char *vendor, char *product,
> -			 bool parsable)
> +static void print_device(const char *busid, const char *vendor,
> +			 const char *product, bool parsable)
>   {
>   	if (parsable)
>   		printf("busid=%s#usbid=%.4s:%.4s#", busid, vendor, product);
> @@ -148,106 +148,71 @@ static void print_product_name(char *product_name, bool parsable)
>   		printf("   %s\n", product_name);
>   }
>
> -static void print_interface(char *busid, char *driver, bool parsable)
> -{
> -	if (parsable)
> -		printf("%s=%s#", busid, driver);
> -	else
> -		printf("%9s%s -> %s\n", "", busid, driver);
> -}
> -
> -static int is_device(void *x)
> -{
> -	struct sysfs_attribute *devpath;
> -	struct sysfs_device *dev = x;
> -	int ret = 0;
> -
> -	devpath = sysfs_get_device_attr(dev, "devpath");
> -	if (devpath && *devpath->value != '0')
> -		ret = 1;
> -
> -	return ret;
> -}
> -
> -static int devcmp(void *a, void *b)
> -{
> -	return strcmp(a, b);
> -}
> -
>   static int list_devices(bool parsable)
>   {
> -	char bus_type[] = "usb";
> -	char busid[SYSFS_BUS_ID_SIZE];
> +	struct udev *udev;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	struct udev_device *dev;
> +	const char *path;
> +	const char *idVendor;
> +	const char *idProduct;
> +	const char *bConfValue;
> +	const char *bNumIntfs;
> +	const char *busid;
>   	char product_name[128];
> -	struct sysfs_bus *ubus;
> -	struct sysfs_device *dev;
> -	struct sysfs_device *intf;
> -	struct sysfs_attribute *idVendor;
> -	struct sysfs_attribute *idProduct;
> -	struct sysfs_attribute *bConfValue;
> -	struct sysfs_attribute *bNumIntfs;
> -	struct dlist *devlist;
> -	int i;
>   	int ret = -1;
>
> -	ubus = sysfs_open_bus(bus_type);
> -	if (!ubus) {
> -		err("could not open %s bus: %s", bus_type, strerror(errno));
> -		return -1;
> -	}
> -
> -	devlist = sysfs_get_bus_devices(ubus);
> -	if (!devlist) {
> -		err("could not get %s bus devices: %s", bus_type,
> -		    strerror(errno));
> -		goto err_out;
> -	}
> -
> -	/* remove interfaces and root hubs from device list */
> -	dlist_filter_sort(devlist, is_device, devcmp);
> -
> -	if (!parsable) {
> -		printf("Local USB devices\n");
> -		printf("=================\n");
> -	}
> -	dlist_for_each_data(devlist, dev, struct sysfs_device) {
> -		idVendor   = sysfs_get_device_attr(dev, "idVendor");
> -		idProduct  = sysfs_get_device_attr(dev, "idProduct");
> -		bConfValue = sysfs_get_device_attr(dev, "bConfigurationValue");
> -		bNumIntfs  = sysfs_get_device_attr(dev, "bNumInterfaces");
> +	/* Create libudev context. */
> +	udev = udev_new();
> +
> +	/* Create libudev device enumeration. */
> +	enumerate = udev_enumerate_new(udev);
> +
> +	/* Take only USB devices that are not hubs and do not have
> +	 * the bInterfaceNumber attribute, i.e. are not interfaces.
> +	 */
> +	udev_enumerate_add_match_subsystem(enumerate, "usb");
> +	udev_enumerate_add_nomatch_sysattr(enumerate, "bDeviceClass", "09");
> +	udev_enumerate_add_nomatch_sysattr(enumerate, "bInterfaceNumber", NULL);
> +	udev_enumerate_scan_devices(enumerate);
> +
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +
> +	/* Show information about each device. */
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		path = udev_list_entry_get_name(dev_list_entry);
> +		dev = udev_device_new_from_syspath(udev, path);
> +
> +		/* Get device information. */
> +		idVendor = udev_device_get_sysattr_value(dev, "idVendor");
> +		idProduct = udev_device_get_sysattr_value(dev, "idProduct");
> +		bConfValue = udev_device_get_sysattr_value(dev, "bConfigurationValue");
> +		bNumIntfs = udev_device_get_sysattr_value(dev, "bNumInterfaces");
> +		busid = udev_device_get_sysname(dev);
>   		if (!idVendor || !idProduct || !bConfValue || !bNumIntfs) {
> -			err("problem getting device attributes: %s",
> +			err("Problem getting device attributes: %s",
>   			    strerror(errno));
>   			goto err_out;
>   		}
>
> -		/* get product name */
> +		/* Get product name. */
>   		usbip_names_get_product(product_name, sizeof(product_name),
> -					strtol(idVendor->value, NULL, 16),
> -					strtol(idProduct->value, NULL, 16));
> -		print_device(dev->bus_id, idVendor->value, idProduct->value,
> -			     parsable);
> +					strtol(idVendor, NULL, 16),
> +					strtol(idProduct, NULL, 16));
> +
> +		/* Print information. */
> +		print_device(busid, idVendor, idProduct, parsable);
>   		print_product_name(product_name, parsable);
>
> -		for (i = 0; i < atoi(bNumIntfs->value); i++) {
> -			snprintf(busid, sizeof(busid), "%s:%.1s.%d",
> -				 dev->bus_id, bConfValue->value, i);
> -			intf = sysfs_open_device(bus_type, busid);
> -			if (!intf) {
> -				err("could not open device interface: %s",
> -				    strerror(errno));
> -				goto err_out;
> -			}
> -			print_interface(busid, intf->driver_name, parsable);
> -			sysfs_close_device(intf);
> -		}
>   		printf("\n");
> -	}
>
> -	ret = 0;
Do you need to delete this? Where does ret get set to 0 then?

> +		udev_device_unref(dev);
> +	}
>
>   err_out:
> -	sysfs_close_bus(ubus);
> +	udev_enumerate_unref(enumerate);
> +	udev_unref(udev);
>
>   	return ret;

Is ret = -1 still at this point?

>   }
>

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

[PATCH 05/12] staging: usbip: userspace: re-add interface information listing

[Valentina Manea]

This was deleted in the driver conversion patch. It didn't need
to be deleted; showing more information is ok.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/src/usbip_list.c | 20 +++++++++++++++++++-
 drivers/staging/usbip/userspace/src/usbipd.c     | 16 +++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_list.c b/drivers/staging/usbip/userspace/src/usbip_list.c
index 2255107..7e1820a 100644
--- a/drivers/staging/usbip/userspace/src/usbip_list.c
+++ b/drivers/staging/usbip/userspace/src/usbip_list.c
@@ -52,8 +52,9 @@ static int get_exported_devices(char *host, int sockfd)
 	struct op_devlist_reply reply;
 	uint16_t code = OP_REP_DEVLIST;
 	struct usbip_usb_device udev;
+	struct usbip_usb_interface uintf;
 	unsigned int i;
-	int rc;
+	int rc, j;
 
 	rc = usbip_net_send_op_common(sockfd, OP_REQ_DEVLIST, 0);
 	if (rc < 0) {
@@ -103,6 +104,23 @@ static int get_exported_devices(char *host, int sockfd)
 		printf("%11s: %s\n", "", udev.path);
 		printf("%11s: %s\n", "", class_name);
 
+		for (j = 0; j < udev.bNumInterfaces; j++) {
+			rc = usbip_net_recv(sockfd, &uintf, sizeof(uintf));
+			if (rc < 0) {
+				dbg("usbip_net_recv failed: usbip_usb_intf[%d]",
+						j);
+
+				return -1;
+			}
+			usbip_net_pack_usb_interface(0, &uintf);
+
+			usbip_names_get_class(class_name, sizeof(class_name),
+					uintf.bInterfaceClass,
+					uintf.bInterfaceSubClass,
+					uintf.bInterfaceProtocol);
+			printf("%11s: %2d - %s\n", "", j, class_name);
+		}
+
 		printf("\n");
 	}
 
diff --git a/drivers/staging/usbip/userspace/src/usbipd.c b/drivers/staging/usbip/userspace/src/usbipd.c
index c2b3ced..b2230f7 100644
--- a/drivers/staging/usbip/userspace/src/usbipd.c
+++ b/drivers/staging/usbip/userspace/src/usbipd.c
@@ -159,8 +159,9 @@ static int send_reply_devlist(int connfd)
 {
 	struct usbip_exported_device *edev;
 	struct usbip_usb_device pdu_udev;
+	struct usbip_usb_interface pdu_uinf;
 	struct op_devlist_reply reply;
-	int rc;
+	int rc, i;
 
 	reply.ndev = 0;
 	/* number of exported devices */
@@ -194,6 +195,19 @@ static int send_reply_devlist(int connfd)
 			dbg("usbip_net_send failed: pdu_udev");
 			return -1;
 		}
+
+		for (i = 0; i < edev->udev.bNumInterfaces; i++) {
+			dump_usb_interface(&edev->uinf[i]);
+			memcpy(&pdu_uinf, &edev->uinf[i], sizeof(pdu_uinf));
+			usbip_net_pack_usb_interface(1, &pdu_uinf);
+
+			rc = usbip_net_send(connfd, &pdu_uinf,
+					sizeof(pdu_uinf));
+			if (rc < 0) {
+				dbg("usbip_net_send failed: pdu_uinf");
+				return -1;
+			}
+		}
 	}
 
 	return 0;
-- 
1.8.1.2

Re: [PATCH 05/12] staging: usbip: userspace: re-add interface information listing

[Shuah Khan]

On 03/04/2014 12:10 PM, Valentina Manea wrote:
> This was deleted in the driver conversion patch. It didn't need
> to be deleted; showing more information is ok.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
> ---
>   drivers/staging/usbip/userspace/src/usbip_list.c | 20 +++++++++++++++++++-
>   drivers/staging/usbip/userspace/src/usbipd.c     | 16 +++++++++++++++-
>   2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/usbip/userspace/src/usbip_list.c b/drivers/staging/usbip/userspace/src/usbip_list.c
> index 2255107..7e1820a 100644
> --- a/drivers/staging/usbip/userspace/src/usbip_list.c
> +++ b/drivers/staging/usbip/userspace/src/usbip_list.c
> @@ -52,8 +52,9 @@ static int get_exported_devices(char *host, int sockfd)
>   	struct op_devlist_reply reply;
>   	uint16_t code = OP_REP_DEVLIST;
>   	struct usbip_usb_device udev;
> +	struct usbip_usb_interface uintf;
>   	unsigned int i;
> -	int rc;
> +	int rc, j;
>
>   	rc = usbip_net_send_op_common(sockfd, OP_REQ_DEVLIST, 0);
>   	if (rc < 0) {
> @@ -103,6 +104,23 @@ static int get_exported_devices(char *host, int sockfd)
>   		printf("%11s: %s\n", "", udev.path);
>   		printf("%11s: %s\n", "", class_name);
>
> +		for (j = 0; j < udev.bNumInterfaces; j++) {
> +			rc = usbip_net_recv(sockfd, &uintf, sizeof(uintf));
> +			if (rc < 0) {
> +				dbg("usbip_net_recv failed: usbip_usb_intf[%d]",
> +						j);

Please use err() instead of dbg() so this error is reported when it 
happens, not just when usbip_use_debug is enabled.

> +
> +				return -1;
> +			}
> +			usbip_net_pack_usb_interface(0, &uintf);
> +
> +			usbip_names_get_class(class_name, sizeof(class_name),
> +					uintf.bInterfaceClass,
> +					uintf.bInterfaceSubClass,
> +					uintf.bInterfaceProtocol);
> +			printf("%11s: %2d - %s\n", "", j, class_name);
> +		}
> +
>   		printf("\n");
>   	}
>
> diff --git a/drivers/staging/usbip/userspace/src/usbipd.c b/drivers/staging/usbip/userspace/src/usbipd.c
> index c2b3ced..b2230f7 100644
> --- a/drivers/staging/usbip/userspace/src/usbipd.c
> +++ b/drivers/staging/usbip/userspace/src/usbipd.c
> @@ -159,8 +159,9 @@ static int send_reply_devlist(int connfd)
>   {
>   	struct usbip_exported_device *edev;
>   	struct usbip_usb_device pdu_udev;
> +	struct usbip_usb_interface pdu_uinf;
>   	struct op_devlist_reply reply;
> -	int rc;
> +	int rc, i;
>
>   	reply.ndev = 0;
>   	/* number of exported devices */
> @@ -194,6 +195,19 @@ static int send_reply_devlist(int connfd)
>   			dbg("usbip_net_send failed: pdu_udev");

Same comment as above.

>   			return -1;
>   		}
> +
> +		for (i = 0; i < edev->udev.bNumInterfaces; i++) {
> +			dump_usb_interface(&edev->uinf[i]);
> +			memcpy(&pdu_uinf, &edev->uinf[i], sizeof(pdu_uinf));
> +			usbip_net_pack_usb_interface(1, &pdu_uinf);
> +
> +			rc = usbip_net_send(connfd, &pdu_uinf,
> +					sizeof(pdu_uinf));
> +			if (rc < 0) {
> +				dbg("usbip_net_send failed: pdu_uinf");

Same comment as above.

> +				return -1;
> +			}
> +		}
>   	}
>
>   	return 0;
>

You have my Reviewed-by after making the recommended changes.

Reviewed-by: Shuah Khan <shuah.kh@samsung.com>

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

[PATCH 06/12] staging: usbip: userspace: add new list API

[Valentina Manea]

Add a new list API from CCAN.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/libsrc/Makefile.am |   3 +-
 .../staging/usbip/userspace/libsrc/build_assert.h  |  40 ++
 .../staging/usbip/userspace/libsrc/check_type.h    |  64 +++
 .../staging/usbip/userspace/libsrc/container_of.h  | 109 ++++
 drivers/staging/usbip/userspace/libsrc/list.c      |  43 ++
 drivers/staging/usbip/userspace/libsrc/list.h      | 613 +++++++++++++++++++++
 6 files changed, 871 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/usbip/userspace/libsrc/build_assert.h
 create mode 100644 drivers/staging/usbip/userspace/libsrc/check_type.h
 create mode 100644 drivers/staging/usbip/userspace/libsrc/container_of.h
 create mode 100644 drivers/staging/usbip/userspace/libsrc/list.c
 create mode 100644 drivers/staging/usbip/userspace/libsrc/list.h

diff --git a/drivers/staging/usbip/userspace/libsrc/Makefile.am b/drivers/staging/usbip/userspace/libsrc/Makefile.am
index 4921189..294270b 100644
--- a/drivers/staging/usbip/userspace/libsrc/Makefile.am
+++ b/drivers/staging/usbip/userspace/libsrc/Makefile.am
@@ -4,4 +4,5 @@ libusbip_la_LDFLAGS  = -version-info @LIBUSBIP_VERSION@
 
 lib_LTLIBRARIES := libusbip.la
 libusbip_la_SOURCES := names.c names.h usbip_host_driver.c usbip_host_driver.h \
-		       usbip_common.c usbip_common.h vhci_driver.c vhci_driver.h
+		       usbip_common.c usbip_common.h vhci_driver.c vhci_driver.h \
+		       list.c list.h build_assert.h check_type.h container_of.h
diff --git a/drivers/staging/usbip/userspace/libsrc/build_assert.h b/drivers/staging/usbip/userspace/libsrc/build_assert.h
new file mode 100644
index 0000000..b9ecd84
--- /dev/null
+++ b/drivers/staging/usbip/userspace/libsrc/build_assert.h
@@ -0,0 +1,40 @@
+/* CC0 (Public domain) - see LICENSE file for details */
+#ifndef CCAN_BUILD_ASSERT_H
+#define CCAN_BUILD_ASSERT_H
+
+/**
+ * BUILD_ASSERT - assert a build-time dependency.
+ * @cond: the compile-time condition which must be true.
+ *
+ * Your compile will fail if the condition isn't true, or can't be evaluated
+ * by the compiler.  This can only be used within a function.
+ *
+ * Example:
+ *	#include <stddef.h>
+ *	...
+ *	static char *foo_to_char(struct foo *foo)
+ *	{
+ *		// This code needs string to be at start of foo.
+ *		BUILD_ASSERT(offsetof(struct foo, string) == 0);
+ *		return (char *)foo;
+ *	}
+ */
+#define BUILD_ASSERT(cond) \
+	do { (void) sizeof(char [1 - 2*!(cond)]); } while(0)
+
+/**
+ * BUILD_ASSERT_OR_ZERO - assert a build-time dependency, as an expression.
+ * @cond: the compile-time condition which must be true.
+ *
+ * Your compile will fail if the condition isn't true, or can't be evaluated
+ * by the compiler.  This can be used in an expression: its value is "0".
+ *
+ * Example:
+ *	#define foo_to_char(foo)					\
+ *		 ((char *)(foo)						\
+ *		  + BUILD_ASSERT_OR_ZERO(offsetof(struct foo, string) == 0))
+ */
+#define BUILD_ASSERT_OR_ZERO(cond) \
+	(sizeof(char [1 - 2*!(cond)]) - 1)
+
+#endif /* CCAN_BUILD_ASSERT_H */
diff --git a/drivers/staging/usbip/userspace/libsrc/check_type.h b/drivers/staging/usbip/userspace/libsrc/check_type.h
new file mode 100644
index 0000000..a39cc41
--- /dev/null
+++ b/drivers/staging/usbip/userspace/libsrc/check_type.h
@@ -0,0 +1,64 @@
+/* CC0 (Public domain) - see LICENSE file for details */
+#ifndef CCAN_CHECK_TYPE_H
+#define CCAN_CHECK_TYPE_H
+#include "config.h"
+
+/**
+ * check_type - issue a warning or build failure if type is not correct.
+ * @expr: the expression whose type we should check (not evaluated).
+ * @type: the exact type we expect the expression to be.
+ *
+ * This macro is usually used within other macros to try to ensure that a macro
+ * argument is of the expected type.  No type promotion of the expression is
+ * done: an unsigned int is not the same as an int!
+ *
+ * check_type() always evaluates to 0.
+ *
+ * If your compiler does not support typeof, then the best we can do is fail
+ * to compile if the sizes of the types are unequal (a less complete check).
+ *
+ * Example:
+ *	// They should always pass a 64-bit value to _set_some_value!
+ *	#define set_some_value(expr)			\
+ *		_set_some_value((check_type((expr), uint64_t), (expr)))
+ */
+
+/**
+ * check_types_match - issue a warning or build failure if types are not same.
+ * @expr1: the first expression (not evaluated).
+ * @expr2: the second expression (not evaluated).
+ *
+ * This macro is usually used within other macros to try to ensure that
+ * arguments are of identical types.  No type promotion of the expressions is
+ * done: an unsigned int is not the same as an int!
+ *
+ * check_types_match() always evaluates to 0.
+ *
+ * If your compiler does not support typeof, then the best we can do is fail
+ * to compile if the sizes of the types are unequal (a less complete check).
+ *
+ * Example:
+ *	// Do subtraction to get to enclosing type, but make sure that
+ *	// pointer is of correct type for that member.
+ *	#define container_of(mbr_ptr, encl_type, mbr)			\
+ *		(check_types_match((mbr_ptr), &((encl_type *)0)->mbr),	\
+ *		 ((encl_type *)						\
+ *		  ((char *)(mbr_ptr) - offsetof(enclosing_type, mbr))))
+ */
+#if HAVE_TYPEOF
+#define check_type(expr, type)			\
+	((typeof(expr) *)0 != (type *)0)
+
+#define check_types_match(expr1, expr2)		\
+	((typeof(expr1) *)0 != (typeof(expr2) *)0)
+#else
+#include <build_assert.h>
+/* Without typeof, we can only test the sizes. */
+#define check_type(expr, type)					\
+	BUILD_ASSERT_OR_ZERO(sizeof(expr) == sizeof(type))
+
+#define check_types_match(expr1, expr2)				\
+	BUILD_ASSERT_OR_ZERO(sizeof(expr1) == sizeof(expr2))
+#endif /* HAVE_TYPEOF */
+
+#endif /* CCAN_CHECK_TYPE_H */
diff --git a/drivers/staging/usbip/userspace/libsrc/container_of.h b/drivers/staging/usbip/userspace/libsrc/container_of.h
new file mode 100644
index 0000000..4afd0fe
--- /dev/null
+++ b/drivers/staging/usbip/userspace/libsrc/container_of.h
@@ -0,0 +1,109 @@
+/* CC0 (Public domain) - see LICENSE file for details */
+#ifndef CCAN_CONTAINER_OF_H
+#define CCAN_CONTAINER_OF_H
+#include <stddef.h>
+
+#include "config.h"
+#include <check_type.h>
+
+/**
+ * container_of - get pointer to enclosing structure
+ * @member_ptr: pointer to the structure member
+ * @containing_type: the type this member is within
+ * @member: the name of this member within the structure.
+ *
+ * Given a pointer to a member of a structure, this macro does pointer
+ * subtraction to return the pointer to the enclosing type.
+ *
+ * Example:
+ *	struct foo {
+ *		int fielda, fieldb;
+ *		// ...
+ *	};
+ *	struct info {
+ *		int some_other_field;
+ *		struct foo my_foo;
+ *	};
+ *
+ *	static struct info *foo_to_info(struct foo *foo)
+ *	{
+ *		return container_of(foo, struct info, my_foo);
+ *	}
+ */
+#define container_of(member_ptr, containing_type, member)		\
+	 ((containing_type *)						\
+	  ((char *)(member_ptr)						\
+	   - container_off(containing_type, member))			\
+	  + check_types_match(*(member_ptr), ((containing_type *)0)->member))
+
+/**
+ * container_off - get offset to enclosing structure
+ * @containing_type: the type this member is within
+ * @member: the name of this member within the structure.
+ *
+ * Given a pointer to a member of a structure, this macro does
+ * typechecking and figures out the offset to the enclosing type.
+ *
+ * Example:
+ *	struct foo {
+ *		int fielda, fieldb;
+ *		// ...
+ *	};
+ *	struct info {
+ *		int some_other_field;
+ *		struct foo my_foo;
+ *	};
+ *
+ *	static struct info *foo_to_info(struct foo *foo)
+ *	{
+ *		size_t off = container_off(struct info, my_foo);
+ *		return (void *)((char *)foo - off);
+ *	}
+ */
+#define container_off(containing_type, member)	\
+	offsetof(containing_type, member)
+
+/**
+ * container_of_var - get pointer to enclosing structure using a variable
+ * @member_ptr: pointer to the structure member
+ * @container_var: a pointer of same type as this member's container
+ * @member: the name of this member within the structure.
+ *
+ * Given a pointer to a member of a structure, this macro does pointer
+ * subtraction to return the pointer to the enclosing type.
+ *
+ * Example:
+ *	static struct info *foo_to_i(struct foo *foo)
+ *	{
+ *		struct info *i = container_of_var(foo, i, my_foo);
+ *		return i;
+ *	}
+ */
+#if HAVE_TYPEOF
+#define container_of_var(member_ptr, container_var, member) \
+	container_of(member_ptr, typeof(*container_var), member)
+#else
+#define container_of_var(member_ptr, container_var, member)	\
+	((void *)((char *)(member_ptr)	-			\
+		  container_off_var(container_var, member)))
+#endif
+
+/**
+ * container_off_var - get offset of a field in enclosing structure
+ * @container_var: a pointer to a container structure
+ * @member: the name of a member within the structure.
+ *
+ * Given (any) pointer to a structure and a its member name, this
+ * macro does pointer subtraction to return offset of member in a
+ * structure memory layout.
+ *
+ */
+#if HAVE_TYPEOF
+#define container_off_var(var, member)		\
+	container_off(typeof(*var), member)
+#else
+#define container_off_var(var, member)			\
+	((char *)&(var)->member - (char *)(var))
+#endif
+
+#endif /* CCAN_CONTAINER_OF_H */
diff --git a/drivers/staging/usbip/userspace/libsrc/list.c b/drivers/staging/usbip/userspace/libsrc/list.c
new file mode 100644
index 0000000..2717fa3
--- /dev/null
+++ b/drivers/staging/usbip/userspace/libsrc/list.c
@@ -0,0 +1,43 @@
+/* Licensed under BSD-MIT - see LICENSE file for details */
+#include <stdio.h>
+#include <stdlib.h>
+#include "list.h"
+
+static void *corrupt(const char *abortstr,
+		     const struct list_node *head,
+		     const struct list_node *node,
+		     unsigned int count)
+{
+	if (abortstr) {
+		fprintf(stderr,
+			"%s: prev corrupt in node %p (%u) of %p\n",
+			abortstr, node, count, head);
+		abort();
+	}
+	return NULL;
+}
+
+struct list_node *list_check_node(const struct list_node *node,
+				  const char *abortstr)
+{
+	const struct list_node *p, *n;
+	int count = 0;
+
+	for (p = node, n = node->next; n != node; p = n, n = n->next) {
+		count++;
+		if (n->prev != p)
+			return corrupt(abortstr, node, n, count);
+	}
+	/* Check prev on head node. */
+	if (node->prev != p)
+		return corrupt(abortstr, node, node, 0);
+
+	return (struct list_node *)node;
+}
+
+struct list_head *list_check(const struct list_head *h, const char *abortstr)
+{
+	if (!list_check_node(&h->n, abortstr))
+		return NULL;
+	return (struct list_head *)h;
+}
diff --git a/drivers/staging/usbip/userspace/libsrc/list.h b/drivers/staging/usbip/userspace/libsrc/list.h
new file mode 100644
index 0000000..dadb40a
--- /dev/null
+++ b/drivers/staging/usbip/userspace/libsrc/list.h
@@ -0,0 +1,613 @@
+/* Licensed under BSD-MIT - see LICENSE file for details */
+#ifndef CCAN_LIST_H
+#define CCAN_LIST_H
+#include <stdbool.h>
+#include <assert.h>
+#include <container_of.h>
+#include <check_type.h>
+
+/**
+ * struct list_node - an entry in a doubly-linked list
+ * @next: next entry (self if empty)
+ * @prev: previous entry (self if empty)
+ *
+ * This is used as an entry in a linked list.
+ * Example:
+ *	struct child {
+ *		const char *name;
+ *		// Linked list of all us children.
+ *		struct list_node list;
+ *	};
+ */
+struct list_node {
+	struct list_node *next, *prev;
+};
+
+/**
+ * struct list_head - the head of a doubly-linked list
+ * @h: the list_head (containing next and prev pointers)
+ *
+ * This is used as the head of a linked list.
+ * Example:
+ *	struct parent {
+ *		const char *name;
+ *		struct list_head children;
+ *		unsigned int num_children;
+ *	};
+ */
+struct list_head {
+	struct list_node n;
+};
+
+/**
+ * list_check - check head of a list for consistency
+ * @h: the list_head
+ * @abortstr: the location to print on aborting, or NULL.
+ *
+ * Because list_nodes have redundant information, consistency checking between
+ * the back and forward links can be done.  This is useful as a debugging check.
+ * If @abortstr is non-NULL, that will be printed in a diagnostic if the list
+ * is inconsistent, and the function will abort.
+ *
+ * Returns the list head if the list is consistent, NULL if not (it
+ * can never return NULL if @abortstr is set).
+ *
+ * See also: list_check_node()
+ *
+ * Example:
+ *	static void dump_parent(struct parent *p)
+ *	{
+ *		struct child *c;
+ *
+ *		printf("%s (%u children):\n", p->name, p->num_children);
+ *		list_check(&p->children, "bad child list");
+ *		list_for_each(&p->children, c, list)
+ *			printf(" -> %s\n", c->name);
+ *	}
+ */
+struct list_head *list_check(const struct list_head *h, const char *abortstr);
+
+/**
+ * list_check_node - check node of a list for consistency
+ * @n: the list_node
+ * @abortstr: the location to print on aborting, or NULL.
+ *
+ * Check consistency of the list node is in (it must be in one).
+ *
+ * See also: list_check()
+ *
+ * Example:
+ *	static void dump_child(const struct child *c)
+ *	{
+ *		list_check_node(&c->list, "bad child list");
+ *		printf("%s\n", c->name);
+ *	}
+ */
+struct list_node *list_check_node(const struct list_node *n,
+				  const char *abortstr);
+
+#ifdef CCAN_LIST_DEBUG
+#define list_debug(h) list_check((h), __func__)
+#define list_debug_node(n) list_check_node((n), __func__)
+#else
+#define list_debug(h) (h)
+#define list_debug_node(n) (n)
+#endif
+
+/**
+ * LIST_HEAD_INIT - initializer for an empty list_head
+ * @name: the name of the list.
+ *
+ * Explicit initializer for an empty list.
+ *
+ * See also:
+ *	LIST_HEAD, list_head_init()
+ *
+ * Example:
+ *	static struct list_head my_list = LIST_HEAD_INIT(my_list);
+ */
+#define LIST_HEAD_INIT(name) { { &name.n, &name.n } }
+
+/**
+ * LIST_HEAD - define and initialize an empty list_head
+ * @name: the name of the list.
+ *
+ * The LIST_HEAD macro defines a list_head and initializes it to an empty
+ * list.  It can be prepended by "static" to define a static list_head.
+ *
+ * See also:
+ *	LIST_HEAD_INIT, list_head_init()
+ *
+ * Example:
+ *	static LIST_HEAD(my_global_list);
+ */
+#define LIST_HEAD(name) \
+	struct list_head name = LIST_HEAD_INIT(name)
+
+/**
+ * list_head_init - initialize a list_head
+ * @h: the list_head to set to the empty list
+ *
+ * Example:
+ *	...
+ *	struct parent *parent = malloc(sizeof(*parent));
+ *
+ *	list_head_init(&parent->children);
+ *	parent->num_children = 0;
+ */
+static inline void list_head_init(struct list_head *h)
+{
+	h->n.next = h->n.prev = &h->n;
+}
+
+/**
+ * list_add - add an entry at the start of a linked list.
+ * @h: the list_head to add the node to
+ * @n: the list_node to add to the list.
+ *
+ * The list_node does not need to be initialized; it will be overwritten.
+ * Example:
+ *	struct child *child = malloc(sizeof(*child));
+ *
+ *	child->name = "marvin";
+ *	list_add(&parent->children, &child->list);
+ *	parent->num_children++;
+ */
+static inline void list_add(struct list_head *h, struct list_node *n)
+{
+	n->next = h->n.next;
+	n->prev = &h->n;
+	h->n.next->prev = n;
+	h->n.next = n;
+	(void)list_debug(h);
+}
+
+/**
+ * list_add_tail - add an entry at the end of a linked list.
+ * @h: the list_head to add the node to
+ * @n: the list_node to add to the list.
+ *
+ * The list_node does not need to be initialized; it will be overwritten.
+ * Example:
+ *	list_add_tail(&parent->children, &child->list);
+ *	parent->num_children++;
+ */
+static inline void list_add_tail(struct list_head *h, struct list_node *n)
+{
+	n->next = &h->n;
+	n->prev = h->n.prev;
+	h->n.prev->next = n;
+	h->n.prev = n;
+	(void)list_debug(h);
+}
+
+/**
+ * list_empty - is a list empty?
+ * @h: the list_head
+ *
+ * If the list is empty, returns true.
+ *
+ * Example:
+ *	assert(list_empty(&parent->children) == (parent->num_children == 0));
+ */
+static inline bool list_empty(const struct list_head *h)
+{
+	(void)list_debug(h);
+	return h->n.next == &h->n;
+}
+
+/**
+ * list_del - delete an entry from an (unknown) linked list.
+ * @n: the list_node to delete from the list.
+ *
+ * Note that this leaves @n in an undefined state; it can be added to
+ * another list, but not deleted again.
+ *
+ * See also:
+ *	list_del_from()
+ *
+ * Example:
+ *	list_del(&child->list);
+ *	parent->num_children--;
+ */
+static inline void list_del(struct list_node *n)
+{
+	(void)list_debug_node(n);
+	n->next->prev = n->prev;
+	n->prev->next = n->next;
+#ifdef CCAN_LIST_DEBUG
+	/* Catch use-after-del. */
+	n->next = n->prev = NULL;
+#endif
+}
+
+/**
+ * list_del_from - delete an entry from a known linked list.
+ * @h: the list_head the node is in.
+ * @n: the list_node to delete from the list.
+ *
+ * This explicitly indicates which list a node is expected to be in,
+ * which is better documentation and can catch more bugs.
+ *
+ * See also: list_del()
+ *
+ * Example:
+ *	list_del_from(&parent->children, &child->list);
+ *	parent->num_children--;
+ */
+static inline void list_del_from(struct list_head *h, struct list_node *n)
+{
+#ifdef CCAN_LIST_DEBUG
+	{
+		/* Thorough check: make sure it was in list! */
+		struct list_node *i;
+		for (i = h->n.next; i != n; i = i->next)
+			assert(i != &h->n);
+	}
+#endif /* CCAN_LIST_DEBUG */
+
+	/* Quick test that catches a surprising number of bugs. */
+	assert(!list_empty(h));
+	list_del(n);
+}
+
+/**
+ * list_entry - convert a list_node back into the structure containing it.
+ * @n: the list_node
+ * @type: the type of the entry
+ * @member: the list_node member of the type
+ *
+ * Example:
+ *	// First list entry is children.next; convert back to child.
+ *	child = list_entry(parent->children.n.next, struct child, list);
+ *
+ * See Also:
+ *	list_top(), list_for_each()
+ */
+#define list_entry(n, type, member) container_of(n, type, member)
+
+/**
+ * list_top - get the first entry in a list
+ * @h: the list_head
+ * @type: the type of the entry
+ * @member: the list_node member of the type
+ *
+ * If the list is empty, returns NULL.
+ *
+ * Example:
+ *	struct child *first;
+ *	first = list_top(&parent->children, struct child, list);
+ *	if (!first)
+ *		printf("Empty list!\n");
+ */
+#define list_top(h, type, member)					\
+	((type *)list_top_((h), list_off_(type, member)))
+
+static inline const void *list_top_(const struct list_head *h, size_t off)
+{
+	if (list_empty(h))
+		return NULL;
+	return (const char *)h->n.next - off;
+}
+
+/**
+ * list_pop - remove the first entry in a list
+ * @h: the list_head
+ * @type: the type of the entry
+ * @member: the list_node member of the type
+ *
+ * If the list is empty, returns NULL.
+ *
+ * Example:
+ *	struct child *one;
+ *	one = list_pop(&parent->children, struct child, list);
+ *	if (!one)
+ *		printf("Empty list!\n");
+ */
+#define list_pop(h, type, member)					\
+	((type *)list_pop_((h), list_off_(type, member)))
+
+static inline const void *list_pop_(const struct list_head *h, size_t off)
+{
+	struct list_node *n;
+
+	if (list_empty(h))
+		return NULL;
+	n = h->n.next;
+	list_del(n);
+	return (const char *)n - off;
+}
+
+/**
+ * list_tail - get the last entry in a list
+ * @h: the list_head
+ * @type: the type of the entry
+ * @member: the list_node member of the type
+ *
+ * If the list is empty, returns NULL.
+ *
+ * Example:
+ *	struct child *last;
+ *	last = list_tail(&parent->children, struct child, list);
+ *	if (!last)
+ *		printf("Empty list!\n");
+ */
+#define list_tail(h, type, member) \
+	((type *)list_tail_((h), list_off_(type, member)))
+
+static inline const void *list_tail_(const struct list_head *h, size_t off)
+{
+	if (list_empty(h))
+		return NULL;
+	return (const char *)h->n.prev - off;
+}
+
+/**
+ * list_for_each - iterate through a list.
+ * @h: the list_head (warning: evaluated multiple times!)
+ * @i: the structure containing the list_node
+ * @member: the list_node member of the structure
+ *
+ * This is a convenient wrapper to iterate @i over the entire list.  It's
+ * a for loop, so you can break and continue as normal.
+ *
+ * Example:
+ *	list_for_each(&parent->children, child, list)
+ *		printf("Name: %s\n", child->name);
+ */
+#define list_for_each(h, i, member)					\
+	list_for_each_off(h, i, list_off_var_(i, member))
+
+/**
+ * list_for_each_rev - iterate through a list backwards.
+ * @h: the list_head
+ * @i: the structure containing the list_node
+ * @member: the list_node member of the structure
+ *
+ * This is a convenient wrapper to iterate @i over the entire list.  It's
+ * a for loop, so you can break and continue as normal.
+ *
+ * Example:
+ *	list_for_each_rev(&parent->children, child, list)
+ *		printf("Name: %s\n", child->name);
+ */
+#define list_for_each_rev(h, i, member)					\
+	for (i = container_of_var(list_debug(h)->n.prev, i, member);	\
+	     &i->member != &(h)->n;					\
+	     i = container_of_var(i->member.prev, i, member))
+
+/**
+ * list_for_each_safe - iterate through a list, maybe during deletion
+ * @h: the list_head
+ * @i: the structure containing the list_node
+ * @nxt: the structure containing the list_node
+ * @member: the list_node member of the structure
+ *
+ * This is a convenient wrapper to iterate @i over the entire list.  It's
+ * a for loop, so you can break and continue as normal.  The extra variable
+ * @nxt is used to hold the next element, so you can delete @i from the list.
+ *
+ * Example:
+ *	struct child *next;
+ *	list_for_each_safe(&parent->children, child, next, list) {
+ *		list_del(&child->list);
+ *		parent->num_children--;
+ *	}
+ */
+#define list_for_each_safe(h, i, nxt, member)				\
+	list_for_each_safe_off(h, i, nxt, list_off_var_(i, member))
+
+/**
+ * list_next - get the next entry in a list
+ * @h: the list_head
+ * @i: a pointer to an entry in the list.
+ * @member: the list_node member of the structure
+ *
+ * If @i was the last entry in the list, returns NULL.
+ *
+ * Example:
+ *	struct child *second;
+ *	second = list_next(&parent->children, first, list);
+ *	if (!second)
+ *		printf("No second child!\n");
+ */
+#define list_next(h, i, member)						\
+	((list_typeof(i))list_entry_or_null(list_debug(h),		\
+					    (i)->member.next,		\
+					    list_off_var_((i), member)))
+
+/**
+ * list_prev - get the previous entry in a list
+ * @h: the list_head
+ * @i: a pointer to an entry in the list.
+ * @member: the list_node member of the structure
+ *
+ * If @i was the first entry in the list, returns NULL.
+ *
+ * Example:
+ *	first = list_prev(&parent->children, second, list);
+ *	if (!first)
+ *		printf("Can't go back to first child?!\n");
+ */
+#define list_prev(h, i, member)						\
+	((list_typeof(i))list_entry_or_null(list_debug(h),		\
+					    (i)->member.prev,		\
+					    list_off_var_((i), member)))
+
+/**
+ * list_append_list - empty one list onto the end of another.
+ * @to: the list to append into
+ * @from: the list to empty.
+ *
+ * This takes the entire contents of @from and moves it to the end of
+ * @to.  After this @from will be empty.
+ *
+ * Example:
+ *	struct list_head adopter;
+ *
+ *	list_append_list(&adopter, &parent->children);
+ *	assert(list_empty(&parent->children));
+ *	parent->num_children = 0;
+ */
+static inline void list_append_list(struct list_head *to,
+				    struct list_head *from)
+{
+	struct list_node *from_tail = list_debug(from)->n.prev;
+	struct list_node *to_tail = list_debug(to)->n.prev;
+
+	/* Sew in head and entire list. */
+	to->n.prev = from_tail;
+	from_tail->next = &to->n;
+	to_tail->next = &from->n;
+	from->n.prev = to_tail;
+
+	/* Now remove head. */
+	list_del(&from->n);
+	list_head_init(from);
+}
+
+/**
+ * list_prepend_list - empty one list into the start of another.
+ * @to: the list to prepend into
+ * @from: the list to empty.
+ *
+ * This takes the entire contents of @from and moves it to the start
+ * of @to.  After this @from will be empty.
+ *
+ * Example:
+ *	list_prepend_list(&adopter, &parent->children);
+ *	assert(list_empty(&parent->children));
+ *	parent->num_children = 0;
+ */
+static inline void list_prepend_list(struct list_head *to,
+				     struct list_head *from)
+{
+	struct list_node *from_tail = list_debug(from)->n.prev;
+	struct list_node *to_head = list_debug(to)->n.next;
+
+	/* Sew in head and entire list. */
+	to->n.next = &from->n;
+	from->n.prev = &to->n;
+	to_head->prev = from_tail;
+	from_tail->next = to_head;
+
+	/* Now remove head. */
+	list_del(&from->n);
+	list_head_init(from);
+}
+
+/**
+ * list_for_each_off - iterate through a list of memory regions.
+ * @h: the list_head
+ * @i: the pointer to a memory region wich contains list node data.
+ * @off: offset(relative to @i) at which list node data resides.
+ *
+ * This is a low-level wrapper to iterate @i over the entire list, used to
+ * implement all oher, more high-level, for-each constructs. It's a for loop,
+ * so you can break and continue as normal.
+ *
+ * WARNING! Being the low-level macro that it is, this wrapper doesn't know
+ * nor care about the type of @i. The only assumtion made is that @i points
+ * to a chunk of memory that at some @offset, relative to @i, contains a
+ * properly filled `struct node_list' which in turn contains pointers to
+ * memory chunks and it's turtles all the way down. Whith all that in mind
+ * remember that given the wrong pointer/offset couple this macro will
+ * happilly churn all you memory untill SEGFAULT stops it, in other words
+ * caveat emptor.
+ *
+ * It is worth mentioning that one of legitimate use-cases for that wrapper
+ * is operation on opaque types with known offset for `struct list_node'
+ * member(preferably 0), because it allows you not to disclose the type of
+ * @i.
+ *
+ * Example:
+ *	list_for_each_off(&parent->children, child,
+ *				offsetof(struct child, list))
+ *		printf("Name: %s\n", child->name);
+ */
+#define list_for_each_off(h, i, off)                                            \
+	for (i = list_node_to_off_(list_debug(h)->n.next, (off));               \
+	     list_node_from_off_((void *)i, (off)) != &(h)->n;                  \
+	     i = list_node_to_off_(list_node_from_off_((void *)i, (off))->next, \
+				   (off)))
+
+/**
+ * list_for_each_safe_off - iterate through a list of memory regions, maybe
+ * during deletion
+ * @h: the list_head
+ * @i: the pointer to a memory region wich contains list node data.
+ * @nxt: the structure containing the list_node
+ * @off: offset(relative to @i) at which list node data resides.
+ *
+ * For details see `list_for_each_off' and `list_for_each_safe'
+ * descriptions.
+ *
+ * Example:
+ *	list_for_each_safe_off(&parent->children, child,
+ *		next, offsetof(struct child, list))
+ *		printf("Name: %s\n", child->name);
+ */
+#define list_for_each_safe_off(h, i, nxt, off)                                \
+	for (i = list_node_to_off_(list_debug(h)->n.next, (off)),             \
+	     nxt = list_node_to_off_(list_node_from_off_(i, (off))->next,     \
+				     (off));                                  \
+	     list_node_from_off_(i, (off)) != &(h)->n;                        \
+	     i = nxt,                                                         \
+		 nxt = list_node_to_off_(list_node_from_off_(i, (off))->next, \
+					 (off)))
+
+
+/* Other -off variants. */
+#define list_entry_off(n, type, off)		\
+	((type *)list_node_from_off_((n), (off)))
+
+#define list_head_off(h, type, off)		\
+	((type *)list_head_off((h), (off)))
+
+#define list_tail_off(h, type, off)		\
+	((type *)list_tail_((h), (off)))
+
+#define list_add_off(h, n, off)                 \
+	list_add((h), list_node_from_off_((n), (off)))
+
+#define list_del_off(n, off)                    \
+	list_del(list_node_from_off_((n), (off)))
+
+#define list_del_from_off(h, n, off)			\
+	list_del_from(h, list_node_from_off_((n), (off)))
+
+/* Offset helper functions so we only single-evaluate. */
+static inline void *list_node_to_off_(struct list_node *node, size_t off)
+{
+	return (void *)((char *)node - off);
+}
+static inline struct list_node *list_node_from_off_(void *ptr, size_t off)
+{
+	return (struct list_node *)((char *)ptr + off);
+}
+
+/* Get the offset of the member, but make sure it's a list_node. */
+#define list_off_(type, member)					\
+	(container_off(type, member) +				\
+	 check_type(((type *)0)->member, struct list_node))
+
+#define list_off_var_(var, member)			\
+	(container_off_var(var, member) +		\
+	 check_type(var->member, struct list_node))
+
+#if HAVE_TYPEOF
+#define list_typeof(var) typeof(var)
+#else
+#define list_typeof(var) void *
+#endif
+
+/* Returns member, or NULL if at end of list. */
+static inline void *list_entry_or_null(const struct list_head *h,
+				       const struct list_node *n,
+				       size_t off)
+{
+	if (n == &h->n)
+		return NULL;
+	return (char *)n - off;
+}
+#endif /* CCAN_LIST_H */
-- 
1.8.1.2

Re: [PATCH 06/12] staging: usbip: userspace: add new list API

[Greg KH]

On Tue, Mar 04, 2014 at 09:10:46PM +0200, Valentina Manea wrote:
> Add a new list API from CCAN.

Why can't you just take the one from the kernel, as userspace is GPLv2
code, right?

And are you sure CC0 is a "valid" license that you can mix with GPLv2
code?  I ask this seriously, as I have heard that CC0 really isn't even
a valid license at all, and if I was to accept this patch, I'm going to
have to go talk to some lawyers, which isn't going to be fun...

thanks,

greg k-h

Re: [PATCH 06/12] staging: usbip: userspace: add new list API

[Dan Carpenter]

On Tue, Mar 04, 2014 at 10:35:39PM -0800, Greg KH wrote:
> On Tue, Mar 04, 2014 at 09:10:46PM +0200, Valentina Manea wrote:
> > Add a new list API from CCAN.
> 
> Why can't you just take the one from the kernel, as userspace is GPLv2
> code, right?
> 
> And are you sure CC0 is a "valid" license that you can mix with GPLv2
> code?  I ask this seriously, as I have heard that CC0 really isn't even
> a valid license at all, and if I was to accept this patch, I'm going to
> have to go talk to some lawyers, which isn't going to be fun...
> 

GNU people say it's compitable.
https://www.gnu.org/licenses/license-list.html#CC0

It's not a good license because it doesn't say anything about that there
is no warranties.  Maybe other reasons as well.

regards,
dan carpenter

Re: [firefly] [PATCH 06/12] staging: usbip: userspace: add new list API

[Greg KH]

On Thu, Mar 06, 2014 at 07:45:27AM +0200, Valentina Manea wrote:
> 
> 
> 
> On Wed, Mar 5, 2014 at 12:16 PM, Dan Carpenter <dan.carpenter@oracle.com>
> wrote:
> 
>     On Tue, Mar 04, 2014 at 10:35:39PM -0800, Greg KH wrote:
>     > On Tue, Mar 04, 2014 at 09:10:46PM +0200, Valentina Manea wrote:
>     > > Add a new list API from CCAN.
>     >
>     > Why can't you just take the one from the kernel, as userspace is GPLv2
>     > code, right?
>     >
>     > And are you sure CC0 is a "valid" license that you can mix with GPLv2
>     > code?  I ask this seriously, as I have heard that CC0 really isn't even
>     > a valid license at all, and if I was to accept this patch, I'm going to
>     > have to go talk to some lawyers, which isn't going to be fun...
>     >
> 
>     GNU people say it's compitable.
>     https://www.gnu.org/licenses/license-list.html#CC0
> 
>     It's not a good license because it doesn't say anything about that there
>     is no warranties.  Maybe other reasons as well.
> 
>     regards,
>     dan carpenter
> 
> 
> 
> As Dan pointed out, it seems that CC0 is compatible with GPLv2.
> However, if needed, I can change the lists API.

You only need the list.h file from the kernel source, strip it down to
just the functions you actually use, and then you should be fine.  Much
smaller codebase to worry about as well, which is always good.

Just be explicit where you copied the file from.

thanks,

greg k-h

[PATCH 07/12] staging: usbip: userspace: move sysfs_utils to libsrc

[Valentina Manea]

Since it offers a API to both usbip tools and libusbip,
it is more appropriate to be place in the library.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/libsrc/Makefile.am |  3 +-
 .../staging/usbip/userspace/libsrc/sysfs_utils.c   | 36 ++++++++++++++++++++++
 .../staging/usbip/userspace/libsrc/sysfs_utils.h   |  8 +++++
 drivers/staging/usbip/userspace/src/Makefile.am    |  3 +-
 drivers/staging/usbip/userspace/src/sysfs_utils.c  | 36 ----------------------
 drivers/staging/usbip/userspace/src/sysfs_utils.h  |  8 -----
 6 files changed, 47 insertions(+), 47 deletions(-)
 create mode 100644 drivers/staging/usbip/userspace/libsrc/sysfs_utils.c
 create mode 100644 drivers/staging/usbip/userspace/libsrc/sysfs_utils.h
 delete mode 100644 drivers/staging/usbip/userspace/src/sysfs_utils.c
 delete mode 100644 drivers/staging/usbip/userspace/src/sysfs_utils.h

diff --git a/drivers/staging/usbip/userspace/libsrc/Makefile.am b/drivers/staging/usbip/userspace/libsrc/Makefile.am
index 294270b..b4c7631 100644
--- a/drivers/staging/usbip/userspace/libsrc/Makefile.am
+++ b/drivers/staging/usbip/userspace/libsrc/Makefile.am
@@ -5,4 +5,5 @@ libusbip_la_LDFLAGS  = -version-info @LIBUSBIP_VERSION@
 lib_LTLIBRARIES := libusbip.la
 libusbip_la_SOURCES := names.c names.h usbip_host_driver.c usbip_host_driver.h \
 		       usbip_common.c usbip_common.h vhci_driver.c vhci_driver.h \
-		       list.c list.h build_assert.h check_type.h container_of.h
+		       list.c list.h build_assert.h check_type.h container_of.h \
+		       sysfs_utils.c sysfs_utils.h
diff --git a/drivers/staging/usbip/userspace/libsrc/sysfs_utils.c b/drivers/staging/usbip/userspace/libsrc/sysfs_utils.c
new file mode 100644
index 0000000..2c362d1
--- /dev/null
+++ b/drivers/staging/usbip/userspace/libsrc/sysfs_utils.c
@@ -0,0 +1,36 @@
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "sysfs_utils.h"
+#include "usbip_common.h"
+
+int write_sysfs_attribute(const char *attr_path, const char *new_value,
+			  size_t len)
+{
+	int fd;
+	int length;
+
+	if (attr_path == NULL || new_value == NULL || len == 0) {
+		dbg("Invalid values provided for attribute %s.", attr_path);
+		errno = EINVAL;
+		return -1;
+	}
+
+	if ((fd = open(attr_path, O_WRONLY)) < 0) {
+		dbg("Error opening attribute %s.", attr_path);
+		return -1;
+	}
+
+	length = write(fd, new_value, len);
+	if (length < 0) {
+		dbg("Error writing to attribute %s.", attr_path);
+		close(fd);
+		return -1;
+	}
+
+	close(fd);
+
+	return 0;
+}
diff --git a/drivers/staging/usbip/userspace/libsrc/sysfs_utils.h b/drivers/staging/usbip/userspace/libsrc/sysfs_utils.h
new file mode 100644
index 0000000..32ac1d1
--- /dev/null
+++ b/drivers/staging/usbip/userspace/libsrc/sysfs_utils.h
@@ -0,0 +1,8 @@
+
+#ifndef __SYSFS_UTILS_H
+#define __SYSFS_UTILS_H
+
+int write_sysfs_attribute(const char *attr_path, const char *new_value,
+			  size_t len);
+
+#endif
diff --git a/drivers/staging/usbip/userspace/src/Makefile.am b/drivers/staging/usbip/userspace/src/Makefile.am
index 6c91bcb..b4f8c4b 100644
--- a/drivers/staging/usbip/userspace/src/Makefile.am
+++ b/drivers/staging/usbip/userspace/src/Makefile.am
@@ -6,8 +6,7 @@ sbin_PROGRAMS := usbip usbipd
 
 usbip_SOURCES := usbip.h utils.h usbip.c utils.c usbip_network.c \
 		 usbip_attach.c usbip_detach.c usbip_list.c \
-		 usbip_bind.c usbip_unbind.c usbip_port.c \
-		 sysfs_utils.c
+		 usbip_bind.c usbip_unbind.c usbip_port.c
 
 
 usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
diff --git a/drivers/staging/usbip/userspace/src/sysfs_utils.c b/drivers/staging/usbip/userspace/src/sysfs_utils.c
deleted file mode 100644
index 2c362d1..0000000
--- a/drivers/staging/usbip/userspace/src/sysfs_utils.c
+++ /dev/null
@@ -1,36 +0,0 @@
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <errno.h>
-
-#include "sysfs_utils.h"
-#include "usbip_common.h"
-
-int write_sysfs_attribute(const char *attr_path, const char *new_value,
-			  size_t len)
-{
-	int fd;
-	int length;
-
-	if (attr_path == NULL || new_value == NULL || len == 0) {
-		dbg("Invalid values provided for attribute %s.", attr_path);
-		errno = EINVAL;
-		return -1;
-	}
-
-	if ((fd = open(attr_path, O_WRONLY)) < 0) {
-		dbg("Error opening attribute %s.", attr_path);
-		return -1;
-	}
-
-	length = write(fd, new_value, len);
-	if (length < 0) {
-		dbg("Error writing to attribute %s.", attr_path);
-		close(fd);
-		return -1;
-	}
-
-	close(fd);
-
-	return 0;
-}
diff --git a/drivers/staging/usbip/userspace/src/sysfs_utils.h b/drivers/staging/usbip/userspace/src/sysfs_utils.h
deleted file mode 100644
index 32ac1d1..0000000
--- a/drivers/staging/usbip/userspace/src/sysfs_utils.h
+++ /dev/null
@@ -1,8 +0,0 @@
-
-#ifndef __SYSFS_UTILS_H
-#define __SYSFS_UTILS_H
-
-int write_sysfs_attribute(const char *attr_path, const char *new_value,
-			  size_t len);
-
-#endif
-- 
1.8.1.2

[PATCH 08/12] staging: usbip: userspace: migrate usbip_host_driver to libudev

[Valentina Manea]

This patch modifies usbip_host_driver to use libudev.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 .../staging/usbip/userspace/libsrc/usbip_common.c  |  74 ++----
 .../staging/usbip/userspace/libsrc/usbip_common.h  |   5 +-
 .../usbip/userspace/libsrc/usbip_host_driver.c     | 282 ++++++---------------
 .../usbip/userspace/libsrc/usbip_host_driver.h     |   7 +-
 .../staging/usbip/userspace/libsrc/vhci_driver.c   |  22 +-
 drivers/staging/usbip/userspace/src/usbipd.c       |  10 +-
 6 files changed, 138 insertions(+), 262 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.c b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
index 6620d18..8d675a9 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.c
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
@@ -2,6 +2,7 @@
  * Copyright (C) 2005-2007 Takahiro Hirofuchi
  */
 
+#include <libudev.h>
 #include "usbip_common.h"
 #include "names.h"
 
@@ -12,6 +13,8 @@ int usbip_use_syslog;
 int usbip_use_stderr;
 int usbip_use_debug;
 
+extern struct udev *udev_context;
+
 struct speed_string {
 	int num;
 	char *speed;
@@ -111,75 +114,48 @@ void dump_usb_device(struct usbip_usb_device *udev)
 }
 
 
-int read_attr_value(struct sysfs_device *dev, const char *name,
+int read_attr_value(struct udev_device *dev, const char *name,
 		    const char *format)
 {
-	char attrpath[SYSFS_PATH_MAX];
-	struct sysfs_attribute *attr;
+	const char *attr;
 	int num = 0;
 	int ret;
 
-	snprintf(attrpath, sizeof(attrpath), "%s/%s", dev->path, name);
-
-	attr = sysfs_open_attribute(attrpath);
+	attr = udev_device_get_sysattr_value(dev, name);
 	if (!attr) {
-		dbg("sysfs_open_attribute failed: %s", attrpath);
-		return 0;
-	}
-
-	ret = sysfs_read_attribute(attr);
-	if (ret < 0) {
-		dbg("sysfs_read_attribute failed");
+		dbg("udev_device_get_sysattr_value failed");
 		goto err;
 	}
 
-	ret = sscanf(attr->value, format, &num);
+	ret = sscanf(attr, format, &num);
 	if (ret < 1) {
 		dbg("sscanf failed");
 		goto err;
 	}
 
 err:
-	sysfs_close_attribute(attr);
 
 	return num;
 }
 
 
-int read_attr_speed(struct sysfs_device *dev)
+int read_attr_speed(struct udev_device *dev)
 {
-	char attrpath[SYSFS_PATH_MAX];
-	struct sysfs_attribute *attr;
-	char speed[100];
-	int ret;
+	const char *speed;
 
-	snprintf(attrpath, sizeof(attrpath), "%s/%s", dev->path, "speed");
-
-	attr = sysfs_open_attribute(attrpath);
-	if (!attr) {
-		dbg("sysfs_open_attribute failed: %s", attrpath);
-		return 0;
-	}
-
-	ret = sysfs_read_attribute(attr);
-	if (ret < 0) {
-		dbg("sysfs_read_attribute failed");
+	speed = udev_device_get_sysattr_value(dev, "speed");
+	if (!speed) {
+		dbg("udev_device_get_sysattr_value failed");
 		goto err;
 	}
 
-	ret = sscanf(attr->value, "%99s\n", speed);
-	if (ret < 1) {
-		dbg("sscanf failed");
-		goto err;
-	}
-err:
-	sysfs_close_attribute(attr);
-
 	for (int i = 0; speed_strings[i].speed != NULL; i++) {
 		if (!strcmp(speed, speed_strings[i].speed))
 			return speed_strings[i].num;
 	}
 
+err:
+
 	return USB_SPEED_UNKNOWN;
 }
 
@@ -190,9 +166,10 @@ err:
 	} while (0)
 
 
-int read_usb_device(struct sysfs_device *sdev, struct usbip_usb_device *udev)
+int read_usb_device(struct udev_device *sdev, struct usbip_usb_device *udev)
 {
 	uint32_t busnum, devnum;
+	const char *path, *name;
 
 	READ_ATTR(udev, uint8_t,  sdev, bDeviceClass,		"%02x\n");
 	READ_ATTR(udev, uint8_t,  sdev, bDeviceSubClass,	"%02x\n");
@@ -209,10 +186,13 @@ int read_usb_device(struct sysfs_device *sdev, struct usbip_usb_device *udev)
 	READ_ATTR(udev, uint8_t,  sdev, devnum,			"%d\n");
 	udev->speed = read_attr_speed(sdev);
 
-	strncpy(udev->path,  sdev->path,  SYSFS_PATH_MAX);
-	strncpy(udev->busid, sdev->name, SYSFS_BUS_ID_SIZE);
+	path = udev_device_get_syspath(sdev);
+	name = udev_device_get_sysname(sdev);
 
-	sscanf(sdev->name, "%u-%u", &busnum, &devnum);
+	strncpy(udev->path,  path,  SYSFS_PATH_MAX);
+	strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE);
+
+	sscanf(name, "%u-%u", &busnum, &devnum);
 	udev->busnum = busnum;
 
 	return 0;
@@ -222,13 +202,13 @@ int read_usb_interface(struct usbip_usb_device *udev, int i,
 		       struct usbip_usb_interface *uinf)
 {
 	char busid[SYSFS_BUS_ID_SIZE];
-	struct sysfs_device *sif;
+	struct udev_device *sif;
 
 	sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i);
 
-	sif = sysfs_open_device("usb", busid);
+	sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid);
 	if (!sif) {
-		dbg("sysfs_open_device(\"usb\", \"%s\") failed", busid);
+		dbg("udev_device_new_from_subsystem_sysname %s failed", busid);
 		return -1;
 	}
 
@@ -236,8 +216,6 @@ int read_usb_interface(struct usbip_usb_device *udev, int i,
 	READ_ATTR(uinf, uint8_t,  sif, bInterfaceSubClass,	"%02x\n");
 	READ_ATTR(uinf, uint8_t,  sif, bInterfaceProtocol,	"%02x\n");
 
-	sysfs_close_device(sif);
-
 	return 0;
 }
 
diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
index 565ac78..9c11060 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
@@ -6,6 +6,7 @@
 #define __USBIP_COMMON_H
 
 #include <sysfs/libsysfs.h>
+#include <libudev.h>
 
 #include <stdint.h>
 #include <stdio.h>
@@ -134,8 +135,8 @@ struct usbip_usb_device {
 
 void dump_usb_interface(struct usbip_usb_interface *);
 void dump_usb_device(struct usbip_usb_device *);
-int read_usb_device(struct sysfs_device *sdev, struct usbip_usb_device *udev);
-int read_attr_value(struct sysfs_device *dev, const char *name,
+int read_usb_device(struct udev_device *sdev, struct usbip_usb_device *udev);
+int read_attr_value(struct udev_device *dev, const char *name,
 		    const char *format);
 int read_usb_interface(struct usbip_usb_device *udev, int i,
 		       struct usbip_usb_interface *uinf);
diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c
index 86a8675..3f34642 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c
@@ -18,101 +18,64 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <fcntl.h>
 
 #include <errno.h>
 #include <unistd.h>
 
+#include <libudev.h>
+
 #include "usbip_common.h"
 #include "usbip_host_driver.h"
+#include "list.h"
+#include "sysfs_utils.h"
 
 #undef  PROGNAME
 #define PROGNAME "libusbip"
 
 struct usbip_host_driver *host_driver;
-
-#define SYSFS_OPEN_RETRIES 100
+struct udev *udev_context;
 
 static int32_t read_attr_usbip_status(struct usbip_usb_device *udev)
 {
-	char attrpath[SYSFS_PATH_MAX];
-	struct sysfs_attribute *attr;
+	char status_attr_path[SYSFS_PATH_MAX];
+	int fd;
+	int length;
+	char status;
 	int value = 0;
-	int rc;
-	struct stat s;
-	int retries = SYSFS_OPEN_RETRIES;
-
-	/* This access is racy!
-	 *
-	 * Just after detach, our driver removes the sysfs
-	 * files and recreates them.
-	 *
-	 * We may try and fail to open the usbip_status of
-	 * an exported device in the (short) window where
-	 * it has been removed and not yet recreated.
-	 *
-	 * This is a bug in the interface. Nothing we can do
-	 * except work around it here by polling for the sysfs
-	 * usbip_status to reappear.
-	 */
-
-	snprintf(attrpath, SYSFS_PATH_MAX, "%s/usbip_status",
-		 udev->path);
-
-	while (retries > 0) {
-		if (stat(attrpath, &s) == 0)
-			break;
-
-		if (errno != ENOENT) {
-			dbg("stat failed: %s", attrpath);
-			return -1;
-		}
 
-		usleep(10000); /* 10ms */
-		retries--;
-	}
-
-	if (retries == 0)
-		dbg("usbip_status not ready after %d retries",
-		    SYSFS_OPEN_RETRIES);
-	else if (retries < SYSFS_OPEN_RETRIES)
-		dbg("warning: usbip_status ready after %d retries",
-		    SYSFS_OPEN_RETRIES - retries);
+	snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status",
+		 udev->path);
 
-	attr = sysfs_open_attribute(attrpath);
-	if (!attr) {
-		dbg("sysfs_open_attribute failed: %s", attrpath);
+	if ((fd = open(status_attr_path, O_RDONLY)) < 0) {
+		dbg("Error opening attribute %s.", status_attr_path);
 		return -1;
 	}
 
-	rc = sysfs_read_attribute(attr);
-	if (rc) {
-		dbg("sysfs_read_attribute failed: %s", attrpath);
-		sysfs_close_attribute(attr);
+	length = read(fd, &status, 1);
+	if (length < 0) {
+		dbg("Error reading attribute %s.", status_attr_path);
+		close(fd);
 		return -1;
 	}
 
-	value = atoi(attr->value);
-
-	sysfs_close_attribute(attr);
+	value = atoi(&status);
 
 	return value;
 }
 
-static struct usbip_exported_device *usbip_exported_device_new(char *sdevpath)
+static
+struct usbip_exported_device *usbip_exported_device_new(const char *sdevpath)
 {
 	struct usbip_exported_device *edev = NULL;
 	size_t size;
 	int i;
 
-	edev = calloc(1, sizeof(*edev));
-	if (!edev) {
-		dbg("calloc failed");
-		return NULL;
-	}
+	edev = calloc(1, sizeof(struct usbip_exported_device));
 
-	edev->sudev = sysfs_open_device_path(sdevpath);
+	edev->sudev = udev_device_new_from_syspath(udev_context, sdevpath);
 	if (!edev->sudev) {
-		dbg("sysfs_open_device_path failed: %s", sdevpath);
+		dbg("udev_device_new_from_syspath: %s", sdevpath);
 		goto err;
 	}
 
@@ -123,130 +86,80 @@ static struct usbip_exported_device *usbip_exported_device_new(char *sdevpath)
 		goto err;
 
 	/* reallocate buffer to include usb interface data */
-	size = sizeof(*edev) + edev->udev.bNumInterfaces *
+	size = sizeof(struct usbip_exported_device) + edev->udev.bNumInterfaces *
 		sizeof(struct usbip_usb_interface);
 
 	edev = realloc(edev, size);
-	if (!edev) {
-		dbg("realloc failed");
-		goto err;
-	}
 
 	for (i = 0; i < edev->udev.bNumInterfaces; i++)
 		read_usb_interface(&edev->udev, i, &edev->uinf[i]);
 
 	return edev;
 err:
-	if (edev && edev->sudev)
-		sysfs_close_device(edev->sudev);
+	if (edev->sudev)
+		udev_device_unref(edev->sudev);
 	if (edev)
 		free(edev);
 
 	return NULL;
 }
 
-static int check_new(struct dlist *dlist, struct sysfs_device *target)
-{
-	struct sysfs_device *dev;
-
-	dlist_for_each_data(dlist, dev, struct sysfs_device) {
-		if (!strncmp(dev->bus_id, target->bus_id, SYSFS_BUS_ID_SIZE))
-			/* device found and is not new */
-			return 0;
-	}
-	return 1;
-}
-
-static void delete_nothing(void *unused_data)
-{
-	/*
-	 * NOTE: Do not delete anything, but the container will be deleted.
-	 */
-	(void) unused_data;
-}
-
 static int refresh_exported_devices(void)
 {
-	/* sysfs_device of usb_device */
-	struct sysfs_device	*sudev;
-	struct dlist		*sudev_list;
-	struct dlist		*sudev_unique_list;
 	struct usbip_exported_device *edev;
-
-	sudev_unique_list = dlist_new_with_delete(sizeof(struct sysfs_device),
-						  delete_nothing);
-
-	sudev_list = sysfs_get_driver_devices(host_driver->sysfs_driver);
-
-	if (!sudev_list) {
-		/*
-		 * Not an error condition. There are simply no devices bound to
-		 * the driver yet.
-		 */
-		dbg("bind " USBIP_HOST_DRV_NAME ".ko to a usb device to be "
-		    "exportable!");
-		return 0;
-	}
-
-	dlist_for_each_data(sudev_list, sudev, struct sysfs_device)
-		if (check_new(sudev_unique_list, sudev))
-			dlist_unshift(sudev_unique_list, sudev);
-
-	dlist_for_each_data(sudev_unique_list, sudev, struct sysfs_device) {
-		edev = usbip_exported_device_new(sudev->path);
-
-		if (!edev) {
-			dbg("usbip_exported_device_new failed");
-			continue;
+	struct udev_enumerate *enumerate;
+	struct udev_list_entry *devices, *dev_list_entry;
+	struct udev_device *dev;
+	const char *path;
+
+	enumerate = udev_enumerate_new(udev_context);
+	udev_enumerate_add_match_subsystem(enumerate, "usb");
+	udev_enumerate_scan_devices(enumerate);
+
+	devices = udev_enumerate_get_list_entry(enumerate);
+
+	udev_list_entry_foreach(dev_list_entry, devices) {
+		path = udev_list_entry_get_name(dev_list_entry);
+		dev = udev_device_new_from_syspath(udev_context, path);
+
+		/* Check whether device uses usbip-host driver. */
+		if (!strcmp(udev_device_get_driver(dev),
+			    USBIP_HOST_DRV_NAME)) {
+			edev = usbip_exported_device_new(path);
+			if (!edev) {
+				dbg("usbip_exported_device_new failed");
+				continue;
+			}
+
+			list_add(&host_driver->edev_list, &edev->node);
+			host_driver->ndevs++;
 		}
-
-		dlist_unshift(host_driver->edev_list, edev);
-		host_driver->ndevs++;
 	}
 
-	dlist_destroy(sudev_unique_list);
-
 	return 0;
 }
 
-static struct sysfs_driver *open_sysfs_host_driver(void)
+static void usbip_exported_device_destroy(void)
 {
-	char bus_type[] = "usb";
-	char sysfs_mntpath[SYSFS_PATH_MAX];
-	char host_drv_path[SYSFS_PATH_MAX];
-	struct sysfs_driver *host_drv;
-	int rc;
-
-	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
-	if (rc < 0) {
-		dbg("sysfs_get_mnt_path failed");
-		return NULL;
-	}
+	struct usbip_exported_device *edev, *edev_next;
 
-	snprintf(host_drv_path, SYSFS_PATH_MAX, "%s/%s/%s/%s/%s",
-		 sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
-		 USBIP_HOST_DRV_NAME);
-
-	host_drv = sysfs_open_driver_path(host_drv_path);
-	if (!host_drv) {
-		dbg("sysfs_open_driver_path failed");
-		return NULL;
+	list_for_each_safe(&host_driver->edev_list, edev,
+			   edev_next, node) {
+		list_del(&edev->node);
+		free(edev);
 	}
-
-	return host_drv;
-}
-
-static void usbip_exported_device_delete(void *dev)
-{
-	struct usbip_exported_device *edev = dev;
-	sysfs_close_device(edev->sudev);
-	free(dev);
 }
 
 int usbip_host_driver_open(void)
 {
 	int rc;
 
+	udev_context = udev_new();
+	if (!udev_context) {
+		dbg("udev_new failed");
+		return -1;
+	}
+
 	host_driver = calloc(1, sizeof(*host_driver));
 	if (!host_driver) {
 		dbg("calloc failed");
@@ -254,32 +167,20 @@ int usbip_host_driver_open(void)
 	}
 
 	host_driver->ndevs = 0;
-	host_driver->edev_list =
-		dlist_new_with_delete(sizeof(struct usbip_exported_device),
-				      usbip_exported_device_delete);
-	if (!host_driver->edev_list) {
-		dbg("dlist_new_with_delete failed");
-		goto err_free_host_driver;
-	}
-
-	host_driver->sysfs_driver = open_sysfs_host_driver();
-	if (!host_driver->sysfs_driver)
-		goto err_destroy_edev_list;
+	list_head_init(&host_driver->edev_list);
 
 	rc = refresh_exported_devices();
 	if (rc < 0)
-		goto err_close_sysfs_driver;
+		goto err_free_host_driver;
 
 	return 0;
 
-err_close_sysfs_driver:
-	sysfs_close_driver(host_driver->sysfs_driver);
-err_destroy_edev_list:
-	dlist_destroy(host_driver->edev_list);
 err_free_host_driver:
 	free(host_driver);
 	host_driver = NULL;
 
+	udev_unref(udev_context);
+
 	return -1;
 }
 
@@ -288,30 +189,22 @@ void usbip_host_driver_close(void)
 	if (!host_driver)
 		return;
 
-	if (host_driver->edev_list)
-		dlist_destroy(host_driver->edev_list);
-	if (host_driver->sysfs_driver)
-		sysfs_close_driver(host_driver->sysfs_driver);
+	usbip_exported_device_destroy();
 
 	free(host_driver);
 	host_driver = NULL;
+
+	udev_unref(udev_context);
 }
 
 int usbip_host_refresh_device_list(void)
 {
 	int rc;
 
-	if (host_driver->edev_list)
-		dlist_destroy(host_driver->edev_list);
+	usbip_exported_device_destroy();
 
 	host_driver->ndevs = 0;
-	host_driver->edev_list =
-		dlist_new_with_delete(sizeof(struct usbip_exported_device),
-				      usbip_exported_device_delete);
-	if (!host_driver->edev_list) {
-		dbg("dlist_new_with_delete failed");
-		return -1;
-	}
+	list_head_init(&host_driver->edev_list);
 
 	rc = refresh_exported_devices();
 	if (rc < 0)
@@ -323,8 +216,7 @@ int usbip_host_refresh_device_list(void)
 int usbip_host_export_device(struct usbip_exported_device *edev, int sockfd)
 {
 	char attr_name[] = "usbip_sockfd";
-	char attr_path[SYSFS_PATH_MAX];
-	struct sysfs_attribute *attr;
+	char sockfd_attr_path[SYSFS_PATH_MAX];
 	char sockfd_buff[30];
 	int ret;
 
@@ -344,40 +236,32 @@ int usbip_host_export_device(struct usbip_exported_device *edev, int sockfd)
 	}
 
 	/* only the first interface is true */
-	snprintf(attr_path, sizeof(attr_path), "%s/%s",
+	snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s",
 		 edev->udev.path, attr_name);
-
-	attr = sysfs_open_attribute(attr_path);
-	if (!attr) {
-		dbg("sysfs_open_attribute failed: %s", attr_path);
-		return -1;
-	}
+	dbg("usbip_sockfd attribute path: %s", sockfd_attr_path);
 
 	snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd);
 	dbg("write: %s", sockfd_buff);
 
-	ret = sysfs_write_attribute(attr, sockfd_buff, strlen(sockfd_buff));
+	ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff,
+				    strlen(sockfd_buff));
 	if (ret < 0) {
-		dbg("sysfs_write_attribute failed: sockfd %s to %s",
-		    sockfd_buff, attr_path);
-		goto err_write_sockfd;
+		dbg("write_sysfs_attribute failed: sockfd %s to %s",
+		    sockfd_buff, sockfd_attr_path);
+		return ret;
 	}
 
 	dbg("connect: %s", edev->udev.busid);
 
-err_write_sockfd:
-	sysfs_close_attribute(attr);
-
 	return ret;
 }
 
 struct usbip_exported_device *usbip_host_get_device(int num)
 {
 	struct usbip_exported_device *edev;
-	struct dlist *dlist = host_driver->edev_list;
 	int cnt = 0;
 
-	dlist_for_each_data(dlist, edev, struct usbip_exported_device) {
+	list_for_each(&host_driver->edev_list, edev, node) {
 		if (num == cnt)
 			return edev;
 		else
diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h
index 34fd14c..8d5ffe3 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h
@@ -21,18 +21,19 @@
 
 #include <stdint.h>
 #include "usbip_common.h"
+#include "list.h"
 
 struct usbip_host_driver {
 	int ndevs;
-	struct sysfs_driver *sysfs_driver;
 	/* list of exported device */
-	struct dlist *edev_list;
+	struct list_head edev_list;
 };
 
 struct usbip_exported_device {
-	struct sysfs_device *sudev;
+	struct udev_device *sudev;
 	int32_t status;
 	struct usbip_usb_device udev;
+	struct list_node node;
 	struct usbip_usb_interface uinf[];
 };
 
diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
index d80d37c..d5839a5 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
@@ -6,24 +6,27 @@
 #include "vhci_driver.h"
 #include <limits.h>
 #include <netdb.h>
+#include <libudev.h>
 
 #undef  PROGNAME
 #define PROGNAME "libusbip"
 
 struct usbip_vhci_driver *vhci_driver;
+struct udev *udev_context;
 
 static struct usbip_imported_device *
 imported_device_init(struct usbip_imported_device *idev, char *busid)
 {
-	struct sysfs_device *sudev;
+	struct udev_device *sudev;
 
-	sudev = sysfs_open_device("usb", busid);
+	sudev = udev_device_new_from_subsystem_sysname(udev_context,
+						       "usb", busid);
 	if (!sudev) {
-		dbg("sysfs_open_device failed: %s", busid);
+		dbg("udev_device_new_from_subsystem_sysname failed: %s", busid);
 		goto err;
 	}
 	read_usb_device(sudev, &idev->udev);
-	sysfs_close_device(sudev);
+	udev_device_unref(sudev);
 
 	/* add class devices of this imported device */
 	struct usbip_class_device *cdev;
@@ -410,6 +413,12 @@ int usbip_vhci_driver_open(void)
 	int ret;
 	char hc_busid[SYSFS_BUS_ID_SIZE];
 
+	udev_context = udev_new();
+	if (!udev_context) {
+		dbg("udev_new failed");
+		return -1;
+	}
+
 	vhci_driver = (struct usbip_vhci_driver *) calloc(1, sizeof(*vhci_driver));
 	if (!vhci_driver) {
 		dbg("calloc failed");
@@ -461,6 +470,9 @@ err:
 		free(vhci_driver);
 
 	vhci_driver = NULL;
+
+	udev_unref(udev_context);
+
 	return -1;
 }
 
@@ -483,6 +495,8 @@ void usbip_vhci_driver_close()
 	free(vhci_driver);
 
 	vhci_driver = NULL;
+
+	udev_unref(udev_context);
 }
 
 
diff --git a/drivers/staging/usbip/userspace/src/usbipd.c b/drivers/staging/usbip/userspace/src/usbipd.c
index b2230f7..9d9360e 100644
--- a/drivers/staging/usbip/userspace/src/usbipd.c
+++ b/drivers/staging/usbip/userspace/src/usbipd.c
@@ -43,6 +43,7 @@
 #include "usbip_host_driver.h"
 #include "usbip_common.h"
 #include "usbip_network.h"
+#include "list.h"
 
 #undef  PROGNAME
 #define PROGNAME "usbipd"
@@ -107,8 +108,7 @@ static int recv_request_import(int sockfd)
 	}
 	PACK_OP_IMPORT_REQUEST(0, &req);
 
-	dlist_for_each_data(host_driver->edev_list, edev,
-			    struct usbip_exported_device) {
+	list_for_each(&host_driver->edev_list, edev, node) {
 		if (!strncmp(req.busid, edev->udev.busid, SYSFS_BUS_ID_SIZE)) {
 			info("found requested device: %s", req.busid);
 			found = 1;
@@ -165,8 +165,7 @@ static int send_reply_devlist(int connfd)
 
 	reply.ndev = 0;
 	/* number of exported devices */
-	dlist_for_each_data(host_driver->edev_list, edev,
-			    struct usbip_exported_device) {
+	list_for_each(&host_driver->edev_list, edev, node) {
 		reply.ndev += 1;
 	}
 	info("exportable devices: %d", reply.ndev);
@@ -184,8 +183,7 @@ static int send_reply_devlist(int connfd)
 		return -1;
 	}
 
-	dlist_for_each_data(host_driver->edev_list, edev,
-			    struct usbip_exported_device) {
+	list_for_each(&host_driver->edev_list, edev, node) {
 		dump_usb_device(&edev->udev);
 		memcpy(&pdu_udev, &edev->udev, sizeof(pdu_udev));
 		usbip_net_pack_usb_device(1, &pdu_udev);
-- 
1.8.1.2

Re: [PATCH 08/12] staging: usbip: userspace: migrate usbip_host_driver to libudev

[Shuah Khan]

On 03/04/2014 12:10 PM, Valentina Manea wrote:
> This patch modifies usbip_host_driver to use libudev.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
> ---
>   .../staging/usbip/userspace/libsrc/usbip_common.c  |  74 ++----
>   .../staging/usbip/userspace/libsrc/usbip_common.h  |   5 +-
>   .../usbip/userspace/libsrc/usbip_host_driver.c     | 282 ++++++---------------
>   .../usbip/userspace/libsrc/usbip_host_driver.h     |   7 +-
>   .../staging/usbip/userspace/libsrc/vhci_driver.c   |  22 +-
>   drivers/staging/usbip/userspace/src/usbipd.c       |  10 +-
>   6 files changed, 138 insertions(+), 262 deletions(-)
>
> diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.c b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
> index 6620d18..8d675a9 100644
> --- a/drivers/staging/usbip/userspace/libsrc/usbip_common.c
> +++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.c
> @@ -2,6 +2,7 @@
>    * Copyright (C) 2005-2007 Takahiro Hirofuchi
>    */
>
> +#include <libudev.h>
>   #include "usbip_common.h"
>   #include "names.h"
>
> @@ -12,6 +13,8 @@ int usbip_use_syslog;
>   int usbip_use_stderr;
>   int usbip_use_debug;
>
> +extern struct udev *udev_context;
> +
>   struct speed_string {
>   	int num;
>   	char *speed;
> @@ -111,75 +114,48 @@ void dump_usb_device(struct usbip_usb_device *udev)
>   }
>
>
> -int read_attr_value(struct sysfs_device *dev, const char *name,
> +int read_attr_value(struct udev_device *dev, const char *name,
>   		    const char *format)
>   {
> -	char attrpath[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *attr;
> +	const char *attr;
>   	int num = 0;
>   	int ret;
>
> -	snprintf(attrpath, sizeof(attrpath), "%s/%s", dev->path, name);
> -
> -	attr = sysfs_open_attribute(attrpath);
> +	attr = udev_device_get_sysattr_value(dev, name);
>   	if (!attr) {
> -		dbg("sysfs_open_attribute failed: %s", attrpath);
> -		return 0;
> -	}
> -
> -	ret = sysfs_read_attribute(attr);
> -	if (ret < 0) {
> -		dbg("sysfs_read_attribute failed");
> +		dbg("udev_device_get_sysattr_value failed");

Please make this an error. Also could you please include device 
information and other useful details in these error messages. Kind of a 
global comment on this patch series.


>   		goto err;
>   	}
>
> -	ret = sscanf(attr->value, format, &num);
> +	ret = sscanf(attr, format, &num);
>   	if (ret < 1) {
>   		dbg("sscanf failed");

Same comment here about error vs. debug.

>   		goto err;
>   	}
>
>   err:
> -	sysfs_close_attribute(attr);
>
>   	return num;
>   }
>
>
> -int read_attr_speed(struct sysfs_device *dev)
> +int read_attr_speed(struct udev_device *dev)
>   {
> -	char attrpath[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *attr;
> -	char speed[100];
> -	int ret;
> +	const char *speed;
>
> -	snprintf(attrpath, sizeof(attrpath), "%s/%s", dev->path, "speed");
> -
> -	attr = sysfs_open_attribute(attrpath);
> -	if (!attr) {
> -		dbg("sysfs_open_attribute failed: %s", attrpath);
> -		return 0;
> -	}
> -
> -	ret = sysfs_read_attribute(attr);
> -	if (ret < 0) {
> -		dbg("sysfs_read_attribute failed");
> +	speed = udev_device_get_sysattr_value(dev, "speed");
> +	if (!speed) {
> +		dbg("udev_device_get_sysattr_value failed");

Same comment here about error vs. debug.

>   		goto err;
>   	}
>
> -	ret = sscanf(attr->value, "%99s\n", speed);
> -	if (ret < 1) {
> -		dbg("sscanf failed");
> -		goto err;
> -	}
> -err:
> -	sysfs_close_attribute(attr);
> -
>   	for (int i = 0; speed_strings[i].speed != NULL; i++) {
>   		if (!strcmp(speed, speed_strings[i].speed))
>   			return speed_strings[i].num;
>   	}
>
> +err:
> +
>   	return USB_SPEED_UNKNOWN;
>   }
>
> @@ -190,9 +166,10 @@ err:
>   	} while (0)
>
>
> -int read_usb_device(struct sysfs_device *sdev, struct usbip_usb_device *udev)
> +int read_usb_device(struct udev_device *sdev, struct usbip_usb_device *udev)
>   {
>   	uint32_t busnum, devnum;
> +	const char *path, *name;
>
>   	READ_ATTR(udev, uint8_t,  sdev, bDeviceClass,		"%02x\n");
>   	READ_ATTR(udev, uint8_t,  sdev, bDeviceSubClass,	"%02x\n");
> @@ -209,10 +186,13 @@ int read_usb_device(struct sysfs_device *sdev, struct usbip_usb_device *udev)
>   	READ_ATTR(udev, uint8_t,  sdev, devnum,			"%d\n");
>   	udev->speed = read_attr_speed(sdev);
>
> -	strncpy(udev->path,  sdev->path,  SYSFS_PATH_MAX);
> -	strncpy(udev->busid, sdev->name, SYSFS_BUS_ID_SIZE);
> +	path = udev_device_get_syspath(sdev);
> +	name = udev_device_get_sysname(sdev);
>
> -	sscanf(sdev->name, "%u-%u", &busnum, &devnum);
> +	strncpy(udev->path,  path,  SYSFS_PATH_MAX);
> +	strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE);
> +
> +	sscanf(name, "%u-%u", &busnum, &devnum);
>   	udev->busnum = busnum;
>
>   	return 0;
> @@ -222,13 +202,13 @@ int read_usb_interface(struct usbip_usb_device *udev, int i,
>   		       struct usbip_usb_interface *uinf)
>   {
>   	char busid[SYSFS_BUS_ID_SIZE];
> -	struct sysfs_device *sif;
> +	struct udev_device *sif;
>
>   	sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i);
>
> -	sif = sysfs_open_device("usb", busid);
> +	sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid);
>   	if (!sif) {
> -		dbg("sysfs_open_device(\"usb\", \"%s\") failed", busid);
> +		dbg("udev_device_new_from_subsystem_sysname %s failed", busid);

Same comment here about error vs. debug.

>   		return -1;
>   	}
>
> @@ -236,8 +216,6 @@ int read_usb_interface(struct usbip_usb_device *udev, int i,
>   	READ_ATTR(uinf, uint8_t,  sif, bInterfaceSubClass,	"%02x\n");
>   	READ_ATTR(uinf, uint8_t,  sif, bInterfaceProtocol,	"%02x\n");
>
> -	sysfs_close_device(sif);
> -
>   	return 0;
>   }
>
> diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> index 565ac78..9c11060 100644
> --- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> +++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> @@ -6,6 +6,7 @@
>   #define __USBIP_COMMON_H
>
>   #include <sysfs/libsysfs.h>
> +#include <libudev.h>
>
>   #include <stdint.h>
>   #include <stdio.h>
> @@ -134,8 +135,8 @@ struct usbip_usb_device {
>
>   void dump_usb_interface(struct usbip_usb_interface *);
>   void dump_usb_device(struct usbip_usb_device *);
> -int read_usb_device(struct sysfs_device *sdev, struct usbip_usb_device *udev);
> -int read_attr_value(struct sysfs_device *dev, const char *name,
> +int read_usb_device(struct udev_device *sdev, struct usbip_usb_device *udev);
> +int read_attr_value(struct udev_device *dev, const char *name,
>   		    const char *format);
>   int read_usb_interface(struct usbip_usb_device *udev, int i,
>   		       struct usbip_usb_interface *uinf);
> diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c
> index 86a8675..3f34642 100644
> --- a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c
> +++ b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.c
> @@ -18,101 +18,64 @@
>
>   #include <sys/types.h>
>   #include <sys/stat.h>
> +#include <fcntl.h>
>
>   #include <errno.h>
>   #include <unistd.h>
>
> +#include <libudev.h>
> +
>   #include "usbip_common.h"
>   #include "usbip_host_driver.h"
> +#include "list.h"
> +#include "sysfs_utils.h"
>
>   #undef  PROGNAME
>   #define PROGNAME "libusbip"
>
>   struct usbip_host_driver *host_driver;
> -
> -#define SYSFS_OPEN_RETRIES 100
> +struct udev *udev_context;
>
>   static int32_t read_attr_usbip_status(struct usbip_usb_device *udev)
>   {
> -	char attrpath[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *attr;
> +	char status_attr_path[SYSFS_PATH_MAX];
> +	int fd;
> +	int length;
> +	char status;
>   	int value = 0;
> -	int rc;
> -	struct stat s;
> -	int retries = SYSFS_OPEN_RETRIES;
> -
> -	/* This access is racy!
> -	 *
> -	 * Just after detach, our driver removes the sysfs
> -	 * files and recreates them.
> -	 *
> -	 * We may try and fail to open the usbip_status of
> -	 * an exported device in the (short) window where
> -	 * it has been removed and not yet recreated.
> -	 *
> -	 * This is a bug in the interface. Nothing we can do
> -	 * except work around it here by polling for the sysfs
> -	 * usbip_status to reappear.
> -	 */
> -
> -	snprintf(attrpath, SYSFS_PATH_MAX, "%s/usbip_status",
> -		 udev->path);
> -
> -	while (retries > 0) {
> -		if (stat(attrpath, &s) == 0)
> -			break;
> -
> -		if (errno != ENOENT) {
> -			dbg("stat failed: %s", attrpath);
> -			return -1;
> -		}
>
> -		usleep(10000); /* 10ms */
> -		retries--;
> -	}
> -
> -	if (retries == 0)
> -		dbg("usbip_status not ready after %d retries",
> -		    SYSFS_OPEN_RETRIES);
> -	else if (retries < SYSFS_OPEN_RETRIES)
> -		dbg("warning: usbip_status ready after %d retries",
> -		    SYSFS_OPEN_RETRIES - retries);
> +	snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status",
> +		 udev->path);
>
> -	attr = sysfs_open_attribute(attrpath);
> -	if (!attr) {
> -		dbg("sysfs_open_attribute failed: %s", attrpath);
> +	if ((fd = open(status_attr_path, O_RDONLY)) < 0) {
> +		dbg("Error opening attribute %s.", status_attr_path);

Same comment here about error vs. debug.

>   		return -1;
>   	}
>
> -	rc = sysfs_read_attribute(attr);
> -	if (rc) {
> -		dbg("sysfs_read_attribute failed: %s", attrpath);
> -		sysfs_close_attribute(attr);
> +	length = read(fd, &status, 1);
> +	if (length < 0) {
> +		dbg("Error reading attribute %s.", status_attr_path);

Same comment here about error vs. debug.

> +		close(fd);
>   		return -1;
>   	}
>
> -	value = atoi(attr->value);
> -
> -	sysfs_close_attribute(attr);
> +	value = atoi(&status);
>
>   	return value;
>   }
>
> -static struct usbip_exported_device *usbip_exported_device_new(char *sdevpath)
> +static
> +struct usbip_exported_device *usbip_exported_device_new(const char *sdevpath)
>   {
>   	struct usbip_exported_device *edev = NULL;
>   	size_t size;
>   	int i;
>
> -	edev = calloc(1, sizeof(*edev));
> -	if (!edev) {
> -		dbg("calloc failed");
> -		return NULL;
> -	}
> +	edev = calloc(1, sizeof(struct usbip_exported_device));
>
> -	edev->sudev = sysfs_open_device_path(sdevpath);
> +	edev->sudev = udev_device_new_from_syspath(udev_context, sdevpath);
>   	if (!edev->sudev) {
> -		dbg("sysfs_open_device_path failed: %s", sdevpath);
> +		dbg("udev_device_new_from_syspath: %s", sdevpath);

Same comment here about error vs. debug.

>   		goto err;
>   	}
>
> @@ -123,130 +86,80 @@ static struct usbip_exported_device *usbip_exported_device_new(char *sdevpath)
>   		goto err;
>
>   	/* reallocate buffer to include usb interface data */
> -	size = sizeof(*edev) + edev->udev.bNumInterfaces *
> +	size = sizeof(struct usbip_exported_device) + edev->udev.bNumInterfaces *
>   		sizeof(struct usbip_usb_interface);
>
>   	edev = realloc(edev, size);
> -	if (!edev) {
> -		dbg("realloc failed");
> -		goto err;
> -	}
>
>   	for (i = 0; i < edev->udev.bNumInterfaces; i++)
>   		read_usb_interface(&edev->udev, i, &edev->uinf[i]);
>
>   	return edev;
>   err:
> -	if (edev && edev->sudev)
> -		sysfs_close_device(edev->sudev);
> +	if (edev->sudev)
> +		udev_device_unref(edev->sudev);
>   	if (edev)
>   		free(edev);
>
>   	return NULL;
>   }
>
> -static int check_new(struct dlist *dlist, struct sysfs_device *target)
> -{
> -	struct sysfs_device *dev;
> -
> -	dlist_for_each_data(dlist, dev, struct sysfs_device) {
> -		if (!strncmp(dev->bus_id, target->bus_id, SYSFS_BUS_ID_SIZE))
> -			/* device found and is not new */
> -			return 0;
> -	}
> -	return 1;
> -}
> -
> -static void delete_nothing(void *unused_data)
> -{
> -	/*
> -	 * NOTE: Do not delete anything, but the container will be deleted.
> -	 */
> -	(void) unused_data;
> -}
> -
>   static int refresh_exported_devices(void)
>   {
> -	/* sysfs_device of usb_device */
> -	struct sysfs_device	*sudev;
> -	struct dlist		*sudev_list;
> -	struct dlist		*sudev_unique_list;
>   	struct usbip_exported_device *edev;
> -
> -	sudev_unique_list = dlist_new_with_delete(sizeof(struct sysfs_device),
> -						  delete_nothing);
> -
> -	sudev_list = sysfs_get_driver_devices(host_driver->sysfs_driver);
> -
> -	if (!sudev_list) {
> -		/*
> -		 * Not an error condition. There are simply no devices bound to
> -		 * the driver yet.
> -		 */
> -		dbg("bind " USBIP_HOST_DRV_NAME ".ko to a usb device to be "
> -		    "exportable!");
> -		return 0;
> -	}
> -
> -	dlist_for_each_data(sudev_list, sudev, struct sysfs_device)
> -		if (check_new(sudev_unique_list, sudev))
> -			dlist_unshift(sudev_unique_list, sudev);
> -
> -	dlist_for_each_data(sudev_unique_list, sudev, struct sysfs_device) {
> -		edev = usbip_exported_device_new(sudev->path);
> -
> -		if (!edev) {
> -			dbg("usbip_exported_device_new failed");
> -			continue;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	struct udev_device *dev;
> +	const char *path;
> +
> +	enumerate = udev_enumerate_new(udev_context);
> +	udev_enumerate_add_match_subsystem(enumerate, "usb");
> +	udev_enumerate_scan_devices(enumerate);
> +
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		path = udev_list_entry_get_name(dev_list_entry);
> +		dev = udev_device_new_from_syspath(udev_context, path);
> +
> +		/* Check whether device uses usbip-host driver. */
> +		if (!strcmp(udev_device_get_driver(dev),
> +			    USBIP_HOST_DRV_NAME)) {
> +			edev = usbip_exported_device_new(path);
> +			if (!edev) {
> +				dbg("usbip_exported_device_new failed");
> +				continue;
> +			}
> +
> +			list_add(&host_driver->edev_list, &edev->node);
> +			host_driver->ndevs++;
>   		}
> -
> -		dlist_unshift(host_driver->edev_list, edev);
> -		host_driver->ndevs++;
>   	}
>
> -	dlist_destroy(sudev_unique_list);
> -
>   	return 0;
>   }
>
> -static struct sysfs_driver *open_sysfs_host_driver(void)
> +static void usbip_exported_device_destroy(void)
>   {
> -	char bus_type[] = "usb";
> -	char sysfs_mntpath[SYSFS_PATH_MAX];
> -	char host_drv_path[SYSFS_PATH_MAX];
> -	struct sysfs_driver *host_drv;
> -	int rc;
> -
> -	rc = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
> -	if (rc < 0) {
> -		dbg("sysfs_get_mnt_path failed");
> -		return NULL;
> -	}
> +	struct usbip_exported_device *edev, *edev_next;
>
> -	snprintf(host_drv_path, SYSFS_PATH_MAX, "%s/%s/%s/%s/%s",
> -		 sysfs_mntpath, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
> -		 USBIP_HOST_DRV_NAME);
> -
> -	host_drv = sysfs_open_driver_path(host_drv_path);
> -	if (!host_drv) {
> -		dbg("sysfs_open_driver_path failed");
> -		return NULL;
> +	list_for_each_safe(&host_driver->edev_list, edev,
> +			   edev_next, node) {
> +		list_del(&edev->node);
> +		free(edev);
>   	}
> -
> -	return host_drv;
> -}
> -
> -static void usbip_exported_device_delete(void *dev)
> -{
> -	struct usbip_exported_device *edev = dev;
> -	sysfs_close_device(edev->sudev);
> -	free(dev);
>   }
>
>   int usbip_host_driver_open(void)
>   {
>   	int rc;
>
> +	udev_context = udev_new();
> +	if (!udev_context) {
> +		dbg("udev_new failed");

Same comment here about error vs. debug.

> +		return -1;
> +	}
> +
>   	host_driver = calloc(1, sizeof(*host_driver));
>   	if (!host_driver) {
>   		dbg("calloc failed");

I know this is old code, but this could be an error.

> @@ -254,32 +167,20 @@ int usbip_host_driver_open(void)
>   	}
>
>   	host_driver->ndevs = 0;
> -	host_driver->edev_list =
> -		dlist_new_with_delete(sizeof(struct usbip_exported_device),
> -				      usbip_exported_device_delete);
> -	if (!host_driver->edev_list) {
> -		dbg("dlist_new_with_delete failed");
> -		goto err_free_host_driver;
> -	}
> -
> -	host_driver->sysfs_driver = open_sysfs_host_driver();
> -	if (!host_driver->sysfs_driver)
> -		goto err_destroy_edev_list;
> +	list_head_init(&host_driver->edev_list);
>
>   	rc = refresh_exported_devices();
>   	if (rc < 0)
> -		goto err_close_sysfs_driver;
> +		goto err_free_host_driver;
>
>   	return 0;
>
> -err_close_sysfs_driver:
> -	sysfs_close_driver(host_driver->sysfs_driver);
> -err_destroy_edev_list:
> -	dlist_destroy(host_driver->edev_list);
>   err_free_host_driver:
>   	free(host_driver);
>   	host_driver = NULL;
>
> +	udev_unref(udev_context);
> +
>   	return -1;
>   }
>
> @@ -288,30 +189,22 @@ void usbip_host_driver_close(void)
>   	if (!host_driver)
>   		return;
>
> -	if (host_driver->edev_list)
> -		dlist_destroy(host_driver->edev_list);
> -	if (host_driver->sysfs_driver)
> -		sysfs_close_driver(host_driver->sysfs_driver);
> +	usbip_exported_device_destroy();
>
>   	free(host_driver);
>   	host_driver = NULL;
> +
> +	udev_unref(udev_context);
>   }
>
>   int usbip_host_refresh_device_list(void)
>   {
>   	int rc;
>
> -	if (host_driver->edev_list)
> -		dlist_destroy(host_driver->edev_list);
> +	usbip_exported_device_destroy();
>
>   	host_driver->ndevs = 0;
> -	host_driver->edev_list =
> -		dlist_new_with_delete(sizeof(struct usbip_exported_device),
> -				      usbip_exported_device_delete);
> -	if (!host_driver->edev_list) {
> -		dbg("dlist_new_with_delete failed");
> -		return -1;
> -	}
> +	list_head_init(&host_driver->edev_list);
>
>   	rc = refresh_exported_devices();
>   	if (rc < 0)
> @@ -323,8 +216,7 @@ int usbip_host_refresh_device_list(void)
>   int usbip_host_export_device(struct usbip_exported_device *edev, int sockfd)
>   {
>   	char attr_name[] = "usbip_sockfd";
> -	char attr_path[SYSFS_PATH_MAX];
> -	struct sysfs_attribute *attr;
> +	char sockfd_attr_path[SYSFS_PATH_MAX];
>   	char sockfd_buff[30];
>   	int ret;
>
> @@ -344,40 +236,32 @@ int usbip_host_export_device(struct usbip_exported_device *edev, int sockfd)
>   	}
>
>   	/* only the first interface is true */
> -	snprintf(attr_path, sizeof(attr_path), "%s/%s",
> +	snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s",
>   		 edev->udev.path, attr_name);
> -
> -	attr = sysfs_open_attribute(attr_path);
> -	if (!attr) {
> -		dbg("sysfs_open_attribute failed: %s", attr_path);
> -		return -1;
> -	}
> +	dbg("usbip_sockfd attribute path: %s", sockfd_attr_path);

You could delete this debug message. Having err() in error legs will 
work well.

>
>   	snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd);
>   	dbg("write: %s", sockfd_buff);

You could delete this debug message.

>
> -	ret = sysfs_write_attribute(attr, sockfd_buff, strlen(sockfd_buff));
> +	ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff,
> +				    strlen(sockfd_buff));
>   	if (ret < 0) {
> -		dbg("sysfs_write_attribute failed: sockfd %s to %s",
> -		    sockfd_buff, attr_path);
> -		goto err_write_sockfd;
> +		dbg("write_sysfs_attribute failed: sockfd %s to %s",
> +		    sockfd_buff, sockfd_attr_path);

Same comment here about error vs. debug.

> +		return ret;
>   	}
>
>   	dbg("connect: %s", edev->udev.busid);

This could be made an info() instead of debug as it indicates a status 
message of connect occuring. Maybe rephrasing would help.

>
> -err_write_sockfd:
> -	sysfs_close_attribute(attr);
> -
>   	return ret;
>   }
>
>   struct usbip_exported_device *usbip_host_get_device(int num)
>   {
>   	struct usbip_exported_device *edev;
> -	struct dlist *dlist = host_driver->edev_list;
>   	int cnt = 0;
>
> -	dlist_for_each_data(dlist, edev, struct usbip_exported_device) {
> +	list_for_each(&host_driver->edev_list, edev, node) {
>   		if (num == cnt)
>   			return edev;
>   		else
> diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h
> index 34fd14c..8d5ffe3 100644
> --- a/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h
> +++ b/drivers/staging/usbip/userspace/libsrc/usbip_host_driver.h
> @@ -21,18 +21,19 @@
>
>   #include <stdint.h>
>   #include "usbip_common.h"
> +#include "list.h"
>
>   struct usbip_host_driver {
>   	int ndevs;
> -	struct sysfs_driver *sysfs_driver;
>   	/* list of exported device */
> -	struct dlist *edev_list;
> +	struct list_head edev_list;
>   };
>
>   struct usbip_exported_device {
> -	struct sysfs_device *sudev;
> +	struct udev_device *sudev;
>   	int32_t status;
>   	struct usbip_usb_device udev;
> +	struct list_node node;
>   	struct usbip_usb_interface uinf[];
>   };
>
> diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
> index d80d37c..d5839a5 100644
> --- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
> +++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
> @@ -6,24 +6,27 @@
>   #include "vhci_driver.h"
>   #include <limits.h>
>   #include <netdb.h>
> +#include <libudev.h>
>
>   #undef  PROGNAME
>   #define PROGNAME "libusbip"
>
>   struct usbip_vhci_driver *vhci_driver;
> +struct udev *udev_context;
>
>   static struct usbip_imported_device *
>   imported_device_init(struct usbip_imported_device *idev, char *busid)
>   {
> -	struct sysfs_device *sudev;
> +	struct udev_device *sudev;
>
> -	sudev = sysfs_open_device("usb", busid);
> +	sudev = udev_device_new_from_subsystem_sysname(udev_context,
> +						       "usb", busid);
>   	if (!sudev) {
> -		dbg("sysfs_open_device failed: %s", busid);
> +		dbg("udev_device_new_from_subsystem_sysname failed: %s", busid);
>   		goto err;
>   	}
>   	read_usb_device(sudev, &idev->udev);
> -	sysfs_close_device(sudev);
> +	udev_device_unref(sudev);
>
>   	/* add class devices of this imported device */
>   	struct usbip_class_device *cdev;
> @@ -410,6 +413,12 @@ int usbip_vhci_driver_open(void)
>   	int ret;
>   	char hc_busid[SYSFS_BUS_ID_SIZE];
>
> +	udev_context = udev_new();
> +	if (!udev_context) {
> +		dbg("udev_new failed");

err() instead of dbg()

> +		return -1;
> +	}
> +
>   	vhci_driver = (struct usbip_vhci_driver *) calloc(1, sizeof(*vhci_driver));
>   	if (!vhci_driver) {
>   		dbg("calloc failed");
> @@ -461,6 +470,9 @@ err:
>   		free(vhci_driver);
>
>   	vhci_driver = NULL;
> +
> +	udev_unref(udev_context);
> +
>   	return -1;
>   }
>
> @@ -483,6 +495,8 @@ void usbip_vhci_driver_close()
>   	free(vhci_driver);
>
>   	vhci_driver = NULL;
> +
> +	udev_unref(udev_context);
>   }
>
>
> diff --git a/drivers/staging/usbip/userspace/src/usbipd.c b/drivers/staging/usbip/userspace/src/usbipd.c
> index b2230f7..9d9360e 100644
> --- a/drivers/staging/usbip/userspace/src/usbipd.c
> +++ b/drivers/staging/usbip/userspace/src/usbipd.c
> @@ -43,6 +43,7 @@
>   #include "usbip_host_driver.h"
>   #include "usbip_common.h"
>   #include "usbip_network.h"
> +#include "list.h"
>
>   #undef  PROGNAME
>   #define PROGNAME "usbipd"
> @@ -107,8 +108,7 @@ static int recv_request_import(int sockfd)
>   	}
>   	PACK_OP_IMPORT_REQUEST(0, &req);
>
> -	dlist_for_each_data(host_driver->edev_list, edev,
> -			    struct usbip_exported_device) {
> +	list_for_each(&host_driver->edev_list, edev, node) {
>   		if (!strncmp(req.busid, edev->udev.busid, SYSFS_BUS_ID_SIZE)) {
>   			info("found requested device: %s", req.busid);
>   			found = 1;
> @@ -165,8 +165,7 @@ static int send_reply_devlist(int connfd)
>
>   	reply.ndev = 0;
>   	/* number of exported devices */
> -	dlist_for_each_data(host_driver->edev_list, edev,
> -			    struct usbip_exported_device) {
> +	list_for_each(&host_driver->edev_list, edev, node) {
>   		reply.ndev += 1;
>   	}
>   	info("exportable devices: %d", reply.ndev);
> @@ -184,8 +183,7 @@ static int send_reply_devlist(int connfd)
>   		return -1;
>   	}
>
> -	dlist_for_each_data(host_driver->edev_list, edev,
> -			    struct usbip_exported_device) {
> +	list_for_each(&host_driver->edev_list, edev, node) {
>   		dump_usb_device(&edev->udev);
>   		memcpy(&pdu_udev, &edev->udev, sizeof(pdu_udev));
>   		usbip_net_pack_usb_device(1, &pdu_udev);
>

You have my Reviewed-by after making the recommended changes.

Reviewed-by: Shuah Khan <shuah.kh@samsung.com>

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

[PATCH 09/12] staging: usbip: userspace: remove class device infrastructure in vhci_driver

[Valentina Manea]

The class device lists were used only when being initialized,
being populated and being destroyed. They had no real meaning
and thus the code was useless.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 .../staging/usbip/userspace/libsrc/vhci_driver.c   | 178 ---------------------
 .../staging/usbip/userspace/libsrc/vhci_driver.h   |   4 -
 2 files changed, 182 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
index d5839a5..73a163aa 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
@@ -28,26 +28,6 @@ imported_device_init(struct usbip_imported_device *idev, char *busid)
 	read_usb_device(sudev, &idev->udev);
 	udev_device_unref(sudev);
 
-	/* add class devices of this imported device */
-	struct usbip_class_device *cdev;
-	dlist_for_each_data(vhci_driver->cdev_list, cdev,
-			    struct usbip_class_device) {
-		if (!strncmp(cdev->dev_path, idev->udev.path,
-			     strlen(idev->udev.path))) {
-			struct usbip_class_device *new_cdev;
-			/*
-			 * alloc and copy because dlist is linked
-			 * from only one list
-			 */
-			new_cdev = calloc(1, sizeof(*new_cdev));
-			if (!new_cdev)
-				goto err;
-
-			memcpy(new_cdev, cdev, sizeof(*new_cdev));
-			dlist_unshift(idev->cdev_list, (void *) new_cdev);
-		}
-	}
-
 	return idev;
 
 err:
@@ -103,12 +83,6 @@ static int parse_status(char *value)
 			idev->busnum	= (devid >> 16);
 			idev->devnum	= (devid & 0x0000ffff);
 
-			idev->cdev_list = dlist_new(sizeof(struct usbip_class_device));
-			if (!idev->cdev_list) {
-				dbg("dlist_new failed");
-				return -1;
-			}
-
 			if (idev->status != VDEV_ST_NULL
 			    && idev->status != VDEV_ST_NOTASSIGNED) {
 				idev = imported_device_init(idev, lbusid);
@@ -132,118 +106,6 @@ static int parse_status(char *value)
 	return 0;
 }
 
-
-static int check_usbip_device(struct sysfs_class_device *cdev)
-{
-	/* /sys/class/video4linux/video0/device */
-	char class_path[SYSFS_PATH_MAX];
-	/* /sys/devices/platform/vhci_hcd/usb6/6-1:1.1 */
-	char dev_path[SYSFS_PATH_MAX];
-	int ret;
-	struct usbip_class_device *usbip_cdev;
-
-	snprintf(class_path, sizeof(class_path), "%s/device", cdev->path);
-
-	ret = sysfs_get_link(class_path, dev_path, sizeof(dev_path));
-	if (ret == 0) {
-		if (!strncmp(dev_path, vhci_driver->hc_device->path,
-			     strlen(vhci_driver->hc_device->path))) {
-			/* found usbip device */
-			usbip_cdev = calloc(1, sizeof(*usbip_cdev));
-			if (!usbip_cdev) {
-				dbg("calloc failed");
-				return -1;
-			}
-			dlist_unshift(vhci_driver->cdev_list, usbip_cdev);
-			strncpy(usbip_cdev->class_path, class_path,
-				sizeof(usbip_cdev->class_path));
-			strncpy(usbip_cdev->dev_path, dev_path,
-				sizeof(usbip_cdev->dev_path));
-			dbg("found: %s %s", class_path, dev_path);
-		}
-	}
-
-	return 0;
-}
-
-
-static int search_class_for_usbip_device(char *cname)
-{
-	struct sysfs_class *class;
-	struct dlist *cdev_list;
-	struct sysfs_class_device *cdev;
-	int ret = 0;
-
-	class = sysfs_open_class(cname);
-	if (!class) {
-		dbg("sysfs_open_class failed");
-		return -1;
-	}
-
-	dbg("class: %s", class->name);
-
-	cdev_list = sysfs_get_class_devices(class);
-	if (!cdev_list)
-		/* nothing */
-		goto out;
-
-	dlist_for_each_data(cdev_list, cdev, struct sysfs_class_device) {
-		dbg("cdev: %s", cdev->name);
-		ret = check_usbip_device(cdev);
-		if (ret < 0)
-			goto out;
-	}
-
-out:
-	sysfs_close_class(class);
-
-	return ret;
-}
-
-
-static int refresh_class_device_list(void)
-{
-	int ret;
-	struct dlist *cname_list;
-	char *cname;
-	char sysfs_mntpath[SYSFS_PATH_MAX];
-	char class_path[SYSFS_PATH_MAX];
-
-	ret = sysfs_get_mnt_path(sysfs_mntpath, SYSFS_PATH_MAX);
-	if (ret < 0) {
-		dbg("sysfs_get_mnt_path failed");
-		return -1;
-	}
-
-	snprintf(class_path, sizeof(class_path), "%s/%s", sysfs_mntpath,
-		 SYSFS_CLASS_NAME);
-
-	/* search under /sys/class */
-	cname_list = sysfs_open_directory_list(class_path);
-	if (!cname_list) {
-		dbg("sysfs_open_directory failed");
-		return -1;
-	}
-
-	dlist_for_each_data(cname_list, cname, char) {
-		ret = search_class_for_usbip_device(cname);
-		if (ret < 0) {
-			sysfs_close_list(cname_list);
-			return -1;
-		}
-	}
-
-	sysfs_close_list(cname_list);
-
-	/* search under /sys/block */
-	ret = search_class_for_usbip_device(SYSFS_BLOCK_NAME);
-	if (ret < 0)
-		return -1;
-
-	return 0;
-}
-
-
 static int refresh_imported_device_list(void)
 {
 	struct sysfs_attribute *attr_status;
@@ -447,13 +309,6 @@ int usbip_vhci_driver_open(void)
 
 	dbg("available ports: %d", vhci_driver->nports);
 
-	vhci_driver->cdev_list = dlist_new(sizeof(struct usbip_class_device));
-	if (!vhci_driver->cdev_list)
-		goto err;
-
-	if (refresh_class_device_list())
-		goto err;
-
 	if (refresh_imported_device_list())
 		goto err;
 
@@ -462,8 +317,6 @@ int usbip_vhci_driver_open(void)
 
 
 err:
-	if (vhci_driver->cdev_list)
-		dlist_destroy(vhci_driver->cdev_list);
 	if (vhci_driver->hc_device)
 		sysfs_close_device(vhci_driver->hc_device);
 	if (vhci_driver)
@@ -482,14 +335,6 @@ void usbip_vhci_driver_close()
 	if (!vhci_driver)
 		return;
 
-	if (vhci_driver->cdev_list)
-		dlist_destroy(vhci_driver->cdev_list);
-
-	for (int i = 0; i < vhci_driver->nports; i++) {
-		if (vhci_driver->idev[i].cdev_list)
-			dlist_destroy(vhci_driver->idev[i].cdev_list);
-	}
-
 	if (vhci_driver->hc_device)
 		sysfs_close_device(vhci_driver->hc_device);
 	free(vhci_driver);
@@ -502,35 +347,12 @@ void usbip_vhci_driver_close()
 
 int usbip_vhci_refresh_device_list(void)
 {
-	if (vhci_driver->cdev_list)
-		dlist_destroy(vhci_driver->cdev_list);
-
-
-	for (int i = 0; i < vhci_driver->nports; i++) {
-		if (vhci_driver->idev[i].cdev_list)
-			dlist_destroy(vhci_driver->idev[i].cdev_list);
-	}
-
-	vhci_driver->cdev_list = dlist_new(sizeof(struct usbip_class_device));
-	if (!vhci_driver->cdev_list)
-		goto err;
-
-	if (refresh_class_device_list())
-		goto err;
 
 	if (refresh_imported_device_list())
 		goto err;
 
 	return 0;
 err:
-	if (vhci_driver->cdev_list)
-		dlist_destroy(vhci_driver->cdev_list);
-
-	for (int i = 0; i < vhci_driver->nports; i++) {
-		if (vhci_driver->idev[i].cdev_list)
-			dlist_destroy(vhci_driver->idev[i].cdev_list);
-	}
-
 	dbg("failed to refresh device list");
 	return -1;
 }
diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
index e071f80..e72baa0 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
@@ -28,7 +28,6 @@ struct usbip_imported_device {
 	uint8_t devnum;
 
 	/* usbip_class_device list */
-	struct dlist *cdev_list;
 	struct usbip_usb_device udev;
 };
 
@@ -38,9 +37,6 @@ struct usbip_vhci_driver {
 	/* /sys/devices/platform/vhci_hcd */
 	struct sysfs_device *hc_device;
 
-	/* usbip_class_device list */
-	struct dlist *cdev_list;
-
 	int nports;
 	struct usbip_imported_device idev[MAXNPORT];
 };
-- 
1.8.1.2

Re: [PATCH 09/12] staging: usbip: userspace: remove class device infrastructure in vhci_driver

[Shuah Khan]

On 03/04/2014 12:10 PM, Valentina Manea wrote:
> The class device lists were used only when being initialized,
> being populated and being destroyed. They had no real meaning
> and thus the code was useless.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
> ---
>   .../staging/usbip/userspace/libsrc/vhci_driver.c   | 178 ---------------------
>   .../staging/usbip/userspace/libsrc/vhci_driver.h   |   4 -
>   2 files changed, 182 deletions(-)
>


After these changes, is there a need for struct usbip_class_device ??

-- Shuah


-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

[PATCH 10/12] staging: usbip: userspace: migrate vhci_driver to libudev

[Valentina Manea]

This patch migrates vhci_driver to libudev.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 .../staging/usbip/userspace/libsrc/usbip_common.h  |   1 -
 .../staging/usbip/userspace/libsrc/vhci_driver.c   | 154 ++++++---------------
 .../staging/usbip/userspace/libsrc/vhci_driver.h   |   5 +-
 3 files changed, 44 insertions(+), 116 deletions(-)

diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
index 9c11060..c4a72c3 100644
--- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
+++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
@@ -5,7 +5,6 @@
 #ifndef __USBIP_COMMON_H
 #define __USBIP_COMMON_H
 
-#include <sysfs/libsysfs.h>
 #include <libudev.h>
 
 #include <stdint.h>
diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
index 73a163aa..e7839a0 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
@@ -7,6 +7,7 @@
 #include <limits.h>
 #include <netdb.h>
 #include <libudev.h>
+#include "sysfs_utils.h"
 
 #undef  PROGNAME
 #define PROGNAME "libusbip"
@@ -36,7 +37,7 @@ err:
 
 
 
-static int parse_status(char *value)
+static int parse_status(const char *value)
 {
 	int ret = 0;
 	char *c;
@@ -108,42 +109,33 @@ static int parse_status(char *value)
 
 static int refresh_imported_device_list(void)
 {
-	struct sysfs_attribute *attr_status;
+	const char *attr_status;
 
-
-	attr_status = sysfs_get_device_attr(vhci_driver->hc_device, "status");
+	attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
+					       "status");
 	if (!attr_status) {
-		dbg("sysfs_get_device_attr(\"status\") failed: %s",
-		    vhci_driver->hc_device->name);
+		dbg("udev_device_get_sysattr_value failed");
 		return -1;
 	}
 
-	dbg("name: %s  path: %s  len: %d  method: %d  value: %s",
-	    attr_status->name, attr_status->path, attr_status->len,
-	    attr_status->method, attr_status->value);
-
-	return parse_status(attr_status->value);
+	return parse_status(attr_status);
 }
 
 static int get_nports(void)
 {
 	char *c;
 	int nports = 0;
-	struct sysfs_attribute *attr_status;
+	const char *attr_status;
 
-	attr_status = sysfs_get_device_attr(vhci_driver->hc_device, "status");
+	attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
+					       "status");
 	if (!attr_status) {
-		dbg("sysfs_get_device_attr(\"status\") failed: %s",
-		    vhci_driver->hc_device->name);
+		dbg("udev_device_get_sysattr_value failed");
 		return -1;
 	}
 
-	dbg("name: %s  path: %s  len: %d  method: %d  value: %s",
-	    attr_status->name, attr_status->path, attr_status->len,
-	    attr_status->method, attr_status->value);
-
 	/* skip a header line */
-	c = strchr(attr_status->value, '\n');
+	c = strchr(attr_status, '\n');
 	if (!c)
 		return 0;
 	c++;
@@ -160,50 +152,6 @@ static int get_nports(void)
 	return nports;
 }
 
-static int get_hc_busid(char *sysfs_mntpath, char *hc_busid)
-{
-	struct sysfs_driver *sdriver;
-	char sdriver_path[SYSFS_PATH_MAX];
-
-	struct sysfs_device *hc_dev;
-	struct dlist *hc_devs;
-
-	int found = 0;
-
-	snprintf(sdriver_path, SYSFS_PATH_MAX, "%s/%s/%s/%s/%s", sysfs_mntpath,
-	SYSFS_BUS_NAME, USBIP_VHCI_BUS_TYPE, SYSFS_DRIVERS_NAME,
-	USBIP_VHCI_DRV_NAME);
-
-	sdriver = sysfs_open_driver_path(sdriver_path);
-	if (!sdriver) {
-		dbg("sysfs_open_driver_path failed: %s", sdriver_path);
-		dbg("make sure " USBIP_CORE_MOD_NAME ".ko and "
-		    USBIP_VHCI_DRV_NAME ".ko are loaded!");
-		return -1;
-	}
-
-	hc_devs = sysfs_get_driver_devices(sdriver);
-	if (!hc_devs) {
-		dbg("sysfs_get_driver failed");
-		goto err;
-	}
-
-	/* assume only one vhci_hcd */
-	dlist_for_each_data(hc_devs, hc_dev, struct sysfs_device) {
-		strncpy(hc_busid, hc_dev->bus_id, SYSFS_BUS_ID_SIZE);
-		found = 1;
-	}
-
-err:
-	sysfs_close_driver(sdriver);
-
-	if (found)
-		return 0;
-
-	dbg("%s not found", hc_busid);
-	return -1;
-}
-
 /*
  * Read the given port's record.
  *
@@ -215,7 +163,6 @@ err:
  */
 static int read_record(int rhport, char *host, unsigned long host_len,
 		char *port, unsigned long port_len, char *busid)
-
 {
 	int part;
 	FILE *file;
@@ -272,36 +219,21 @@ static int read_record(int rhport, char *host, unsigned long host_len,
 
 int usbip_vhci_driver_open(void)
 {
-	int ret;
-	char hc_busid[SYSFS_BUS_ID_SIZE];
-
 	udev_context = udev_new();
 	if (!udev_context) {
 		dbg("udev_new failed");
 		return -1;
 	}
 
-	vhci_driver = (struct usbip_vhci_driver *) calloc(1, sizeof(*vhci_driver));
-	if (!vhci_driver) {
-		dbg("calloc failed");
-		return -1;
-	}
-
-	ret = sysfs_get_mnt_path(vhci_driver->sysfs_mntpath, SYSFS_PATH_MAX);
-	if (ret < 0) {
-		dbg("sysfs_get_mnt_path failed");
-		goto err;
-	}
-
-	ret = get_hc_busid(vhci_driver->sysfs_mntpath, hc_busid);
-	if (ret < 0)
-		goto err;
+	vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver));
 
 	/* will be freed in usbip_driver_close() */
-	vhci_driver->hc_device = sysfs_open_device(USBIP_VHCI_BUS_TYPE,
-						   hc_busid);
+	vhci_driver->hc_device =
+		udev_device_new_from_subsystem_sysname(udev_context,
+						       USBIP_VHCI_BUS_TYPE,
+						       USBIP_VHCI_DRV_NAME);
 	if (!vhci_driver->hc_device) {
-		dbg("sysfs_open_device failed");
+		dbg("udev_device_new_from_subsystem_sysname");
 		goto err;
 	}
 
@@ -312,13 +244,11 @@ int usbip_vhci_driver_open(void)
 	if (refresh_imported_device_list())
 		goto err;
 
-
 	return 0;
 
-
 err:
-	if (vhci_driver->hc_device)
-		sysfs_close_device(vhci_driver->hc_device);
+	udev_device_unref(vhci_driver->hc_device);
+
 	if (vhci_driver)
 		free(vhci_driver);
 
@@ -335,8 +265,8 @@ void usbip_vhci_driver_close()
 	if (!vhci_driver)
 		return;
 
-	if (vhci_driver->hc_device)
-		sysfs_close_device(vhci_driver->hc_device);
+	udev_device_unref(vhci_driver->hc_device);
+
 	free(vhci_driver);
 
 	vhci_driver = NULL;
@@ -370,24 +300,24 @@ int usbip_vhci_get_free_port(void)
 
 int usbip_vhci_attach_device2(uint8_t port, int sockfd, uint32_t devid,
 		uint32_t speed) {
-	struct sysfs_attribute *attr_attach;
 	char buff[200]; /* what size should be ? */
+	char attach_attr_path[SYSFS_PATH_MAX];
+	char attr_attach[] = "attach";
+	const char *path;
 	int ret;
 
-	attr_attach = sysfs_get_device_attr(vhci_driver->hc_device, "attach");
-	if (!attr_attach) {
-		dbg("sysfs_get_device_attr(\"attach\") failed: %s",
-		    vhci_driver->hc_device->name);
-		return -1;
-	}
-
 	snprintf(buff, sizeof(buff), "%u %d %u %u",
 			port, sockfd, devid, speed);
 	dbg("writing: %s", buff);
 
-	ret = sysfs_write_attribute(attr_attach, buff, strlen(buff));
+	path = udev_device_get_syspath(vhci_driver->hc_device);
+	snprintf(attach_attr_path, sizeof(attach_attr_path), "%s/%s",
+		 path, attr_attach);
+	dbg("attach attribute path: %s", attach_attr_path);
+
+	ret = write_sysfs_attribute(attach_attr_path, buff, strlen(buff));
 	if (ret < 0) {
-		dbg("sysfs_write_attribute failed");
+		dbg("write_sysfs_attribute failed");
 		return -1;
 	}
 
@@ -412,23 +342,23 @@ int usbip_vhci_attach_device(uint8_t port, int sockfd, uint8_t busnum,
 
 int usbip_vhci_detach_device(uint8_t port)
 {
-	struct sysfs_attribute  *attr_detach;
+	char detach_attr_path[SYSFS_PATH_MAX];
+	char attr_detach[] = "detach";
 	char buff[200]; /* what size should be ? */
+	const char *path;
 	int ret;
 
-	attr_detach = sysfs_get_device_attr(vhci_driver->hc_device, "detach");
-	if (!attr_detach) {
-		dbg("sysfs_get_device_attr(\"detach\") failed: %s",
-		    vhci_driver->hc_device->name);
-		return -1;
-	}
-
 	snprintf(buff, sizeof(buff), "%u", port);
 	dbg("writing: %s", buff);
 
-	ret = sysfs_write_attribute(attr_detach, buff, strlen(buff));
+	path = udev_device_get_syspath(vhci_driver->hc_device);
+	snprintf(detach_attr_path, sizeof(detach_attr_path), "%s/%s",
+		 path, attr_detach);
+	dbg("detach attribute path: %s", detach_attr_path);
+
+	ret = write_sysfs_attribute(detach_attr_path, buff, strlen(buff));
 	if (ret < 0) {
-		dbg("sysfs_write_attribute failed");
+		dbg("write_sysfs_attribute failed");
 		return -1;
 	}
 
diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
index e72baa0..8a84fdf 100644
--- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
+++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
@@ -5,7 +5,7 @@
 #ifndef __VHCI_DRIVER_H
 #define __VHCI_DRIVER_H
 
-#include <sysfs/libsysfs.h>
+#include <libudev.h>
 #include <stdint.h>
 
 #include "usbip_common.h"
@@ -32,10 +32,9 @@ struct usbip_imported_device {
 };
 
 struct usbip_vhci_driver {
-	char sysfs_mntpath[SYSFS_PATH_MAX];
 
 	/* /sys/devices/platform/vhci_hcd */
-	struct sysfs_device *hc_device;
+	struct udev_device *hc_device;
 
 	int nports;
 	struct usbip_imported_device idev[MAXNPORT];
-- 
1.8.1.2

Re: [PATCH 10/12] staging: usbip: userspace: migrate vhci_driver to libudev

[Shuah Khan]

On 03/04/2014 12:10 PM, Valentina Manea wrote:
> This patch migrates vhci_driver to libudev.
>
> Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
> ---
>   .../staging/usbip/userspace/libsrc/usbip_common.h  |   1 -
>   .../staging/usbip/userspace/libsrc/vhci_driver.c   | 154 ++++++---------------
>   .../staging/usbip/userspace/libsrc/vhci_driver.h   |   5 +-
>   3 files changed, 44 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/staging/usbip/userspace/libsrc/usbip_common.h b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> index 9c11060..c4a72c3 100644
> --- a/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> +++ b/drivers/staging/usbip/userspace/libsrc/usbip_common.h
> @@ -5,7 +5,6 @@
>   #ifndef __USBIP_COMMON_H
>   #define __USBIP_COMMON_H
>
> -#include <sysfs/libsysfs.h>
>   #include <libudev.h>
>
>   #include <stdint.h>
> diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
> index 73a163aa..e7839a0 100644
> --- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
> +++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.c
> @@ -7,6 +7,7 @@
>   #include <limits.h>
>   #include <netdb.h>
>   #include <libudev.h>
> +#include "sysfs_utils.h"
>
>   #undef  PROGNAME
>   #define PROGNAME "libusbip"
> @@ -36,7 +37,7 @@ err:
>
>
>
> -static int parse_status(char *value)
> +static int parse_status(const char *value)
>   {
>   	int ret = 0;
>   	char *c;
> @@ -108,42 +109,33 @@ static int parse_status(char *value)
>
>   static int refresh_imported_device_list(void)
>   {
> -	struct sysfs_attribute *attr_status;
> +	const char *attr_status;
>
> -
> -	attr_status = sysfs_get_device_attr(vhci_driver->hc_device, "status");
> +	attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
> +					       "status");
>   	if (!attr_status) {
> -		dbg("sysfs_get_device_attr(\"status\") failed: %s",
> -		    vhci_driver->hc_device->name);
> +		dbg("udev_device_get_sysattr_value failed");

I am afraid you are going to be tired of me saying this :) err() please. 
I have had to run usbip userspace tools with strace a few times to 
figure out where it failed. Hence this insistence on err() where it is 
makes sense.

>   		return -1;
>   	}
>
> -	dbg("name: %s  path: %s  len: %d  method: %d  value: %s",
> -	    attr_status->name, attr_status->path, attr_status->len,
> -	    attr_status->method, attr_status->value);
> -
> -	return parse_status(attr_status->value);
> +	return parse_status(attr_status);
>   }
>
>   static int get_nports(void)
>   {
>   	char *c;
>   	int nports = 0;
> -	struct sysfs_attribute *attr_status;
> +	const char *attr_status;
>
> -	attr_status = sysfs_get_device_attr(vhci_driver->hc_device, "status");
> +	attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
> +					       "status");
>   	if (!attr_status) {
> -		dbg("sysfs_get_device_attr(\"status\") failed: %s",
> -		    vhci_driver->hc_device->name);
> +		dbg("udev_device_get_sysattr_value failed");

Same here err()

>   		return -1;
>   	}
>
> -	dbg("name: %s  path: %s  len: %d  method: %d  value: %s",
> -	    attr_status->name, attr_status->path, attr_status->len,
> -	    attr_status->method, attr_status->value);
> -
>   	/* skip a header line */
> -	c = strchr(attr_status->value, '\n');
> +	c = strchr(attr_status, '\n');
>   	if (!c)
>   		return 0;
>   	c++;
> @@ -160,50 +152,6 @@ static int get_nports(void)
>   	return nports;
>   }
>
> -static int get_hc_busid(char *sysfs_mntpath, char *hc_busid)
> -{
> -	struct sysfs_driver *sdriver;
> -	char sdriver_path[SYSFS_PATH_MAX];
> -
> -	struct sysfs_device *hc_dev;
> -	struct dlist *hc_devs;
> -
> -	int found = 0;
> -
> -	snprintf(sdriver_path, SYSFS_PATH_MAX, "%s/%s/%s/%s/%s", sysfs_mntpath,
> -	SYSFS_BUS_NAME, USBIP_VHCI_BUS_TYPE, SYSFS_DRIVERS_NAME,
> -	USBIP_VHCI_DRV_NAME);
> -
> -	sdriver = sysfs_open_driver_path(sdriver_path);
> -	if (!sdriver) {
> -		dbg("sysfs_open_driver_path failed: %s", sdriver_path);
> -		dbg("make sure " USBIP_CORE_MOD_NAME ".ko and "
> -		    USBIP_VHCI_DRV_NAME ".ko are loaded!");
> -		return -1;
> -	}
> -
> -	hc_devs = sysfs_get_driver_devices(sdriver);
> -	if (!hc_devs) {
> -		dbg("sysfs_get_driver failed");
> -		goto err;
> -	}
> -
> -	/* assume only one vhci_hcd */
> -	dlist_for_each_data(hc_devs, hc_dev, struct sysfs_device) {
> -		strncpy(hc_busid, hc_dev->bus_id, SYSFS_BUS_ID_SIZE);
> -		found = 1;
> -	}
> -
> -err:
> -	sysfs_close_driver(sdriver);
> -
> -	if (found)
> -		return 0;
> -
> -	dbg("%s not found", hc_busid);
> -	return -1;
> -}
> -
>   /*
>    * Read the given port's record.
>    *
> @@ -215,7 +163,6 @@ err:
>    */
>   static int read_record(int rhport, char *host, unsigned long host_len,
>   		char *port, unsigned long port_len, char *busid)
> -
>   {
>   	int part;
>   	FILE *file;
> @@ -272,36 +219,21 @@ static int read_record(int rhport, char *host, unsigned long host_len,
>
>   int usbip_vhci_driver_open(void)
>   {
> -	int ret;
> -	char hc_busid[SYSFS_BUS_ID_SIZE];
> -
>   	udev_context = udev_new();
>   	if (!udev_context) {
>   		dbg("udev_new failed");
>   		return -1;
>   	}
>
> -	vhci_driver = (struct usbip_vhci_driver *) calloc(1, sizeof(*vhci_driver));
> -	if (!vhci_driver) {
> -		dbg("calloc failed");
> -		return -1;
> -	}
> -
> -	ret = sysfs_get_mnt_path(vhci_driver->sysfs_mntpath, SYSFS_PATH_MAX);
> -	if (ret < 0) {
> -		dbg("sysfs_get_mnt_path failed");
> -		goto err;
> -	}
> -
> -	ret = get_hc_busid(vhci_driver->sysfs_mntpath, hc_busid);
> -	if (ret < 0)
> -		goto err;
> +	vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver));
>
>   	/* will be freed in usbip_driver_close() */
> -	vhci_driver->hc_device = sysfs_open_device(USBIP_VHCI_BUS_TYPE,
> -						   hc_busid);
> +	vhci_driver->hc_device =
> +		udev_device_new_from_subsystem_sysname(udev_context,
> +						       USBIP_VHCI_BUS_TYPE,
> +						       USBIP_VHCI_DRV_NAME);
>   	if (!vhci_driver->hc_device) {
> -		dbg("sysfs_open_device failed");
> +		dbg("udev_device_new_from_subsystem_sysname");

Same comment about error

>   		goto err;
>   	}
>
> @@ -312,13 +244,11 @@ int usbip_vhci_driver_open(void)
>   	if (refresh_imported_device_list())
>   		goto err;
>
> -
>   	return 0;
>
> -
>   err:
> -	if (vhci_driver->hc_device)
> -		sysfs_close_device(vhci_driver->hc_device);
> +	udev_device_unref(vhci_driver->hc_device);
> +
>   	if (vhci_driver)
>   		free(vhci_driver);
>
> @@ -335,8 +265,8 @@ void usbip_vhci_driver_close()
>   	if (!vhci_driver)
>   		return;
>
> -	if (vhci_driver->hc_device)
> -		sysfs_close_device(vhci_driver->hc_device);
> +	udev_device_unref(vhci_driver->hc_device);
> +
>   	free(vhci_driver);
>
>   	vhci_driver = NULL;
> @@ -370,24 +300,24 @@ int usbip_vhci_get_free_port(void)
>
>   int usbip_vhci_attach_device2(uint8_t port, int sockfd, uint32_t devid,
>   		uint32_t speed) {
> -	struct sysfs_attribute *attr_attach;
>   	char buff[200]; /* what size should be ? */
> +	char attach_attr_path[SYSFS_PATH_MAX];
> +	char attr_attach[] = "attach";
> +	const char *path;
>   	int ret;
>
> -	attr_attach = sysfs_get_device_attr(vhci_driver->hc_device, "attach");
> -	if (!attr_attach) {
> -		dbg("sysfs_get_device_attr(\"attach\") failed: %s",
> -		    vhci_driver->hc_device->name);
> -		return -1;
> -	}
> -
>   	snprintf(buff, sizeof(buff), "%u %d %u %u",
>   			port, sockfd, devid, speed);
>   	dbg("writing: %s", buff);
>
> -	ret = sysfs_write_attribute(attr_attach, buff, strlen(buff));
> +	path = udev_device_get_syspath(vhci_driver->hc_device);
> +	snprintf(attach_attr_path, sizeof(attach_attr_path), "%s/%s",
> +		 path, attr_attach);
> +	dbg("attach attribute path: %s", attach_attr_path);
> +
> +	ret = write_sysfs_attribute(attach_attr_path, buff, strlen(buff));
>   	if (ret < 0) {
> -		dbg("sysfs_write_attribute failed");
> +		dbg("write_sysfs_attribute failed");
>   		return -1;
>   	}
>
> @@ -412,23 +342,23 @@ int usbip_vhci_attach_device(uint8_t port, int sockfd, uint8_t busnum,
>
>   int usbip_vhci_detach_device(uint8_t port)
>   {
> -	struct sysfs_attribute  *attr_detach;
> +	char detach_attr_path[SYSFS_PATH_MAX];
> +	char attr_detach[] = "detach";
>   	char buff[200]; /* what size should be ? */
> +	const char *path;
>   	int ret;
>
> -	attr_detach = sysfs_get_device_attr(vhci_driver->hc_device, "detach");
> -	if (!attr_detach) {
> -		dbg("sysfs_get_device_attr(\"detach\") failed: %s",
> -		    vhci_driver->hc_device->name);
> -		return -1;
> -	}
> -
>   	snprintf(buff, sizeof(buff), "%u", port);
>   	dbg("writing: %s", buff);
>
> -	ret = sysfs_write_attribute(attr_detach, buff, strlen(buff));
> +	path = udev_device_get_syspath(vhci_driver->hc_device);
> +	snprintf(detach_attr_path, sizeof(detach_attr_path), "%s/%s",
> +		 path, attr_detach);
> +	dbg("detach attribute path: %s", detach_attr_path);
> +
> +	ret = write_sysfs_attribute(detach_attr_path, buff, strlen(buff));
>   	if (ret < 0) {
> -		dbg("sysfs_write_attribute failed");
> +		dbg("write_sysfs_attribute failed");
>   		return -1;
>   	}
>
> diff --git a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
> index e72baa0..8a84fdf 100644
> --- a/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
> +++ b/drivers/staging/usbip/userspace/libsrc/vhci_driver.h
> @@ -5,7 +5,7 @@
>   #ifndef __VHCI_DRIVER_H
>   #define __VHCI_DRIVER_H
>
> -#include <sysfs/libsysfs.h>
> +#include <libudev.h>
>   #include <stdint.h>
>
>   #include "usbip_common.h"
> @@ -32,10 +32,9 @@ struct usbip_imported_device {
>   };
>
>   struct usbip_vhci_driver {
> -	char sysfs_mntpath[SYSFS_PATH_MAX];
>
>   	/* /sys/devices/platform/vhci_hcd */
> -	struct sysfs_device *hc_device;
> +	struct udev_device *hc_device;
>
>   	int nports;
>   	struct usbip_imported_device idev[MAXNPORT];
>

You have my Reviewed-by after making the recommended changes.

Reviewed-by: Shuah Khan <shuah.kh@samsung.com>

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

[PATCH 11/12] staging: usbip: userspace: remove libsysfs flag and autoconf check

[Valentina Manea]

libsysfs is now completely removed from USB/IP.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/configure.ac | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/usbip/userspace/configure.ac b/drivers/staging/usbip/userspace/configure.ac
index a5193c6..25bf160 100644
--- a/drivers/staging/usbip/userspace/configure.ac
+++ b/drivers/staging/usbip/userspace/configure.ac
@@ -44,12 +44,6 @@ AC_FUNC_REALLOC
 AC_CHECK_FUNCS([memset mkdir regcomp socket strchr strerror strstr dnl
 		strtoul])
 
-AC_CHECK_HEADER([sysfs/libsysfs.h],
-		[AC_CHECK_LIB([sysfs], [sysfs_open_directory_list],
-			      [LIBS="$LIBS -lsysfs"],
-			      [AC_MSG_ERROR([Missing sysfs2 library!])])],
-		[AC_MSG_ERROR([Missing /usr/include/sysfs/libsysfs.h])])
-
 AC_CHECK_HEADER([libudev.h],
 		[AC_CHECK_LIB([udev], [udev_new],
 			      [LIBS="$LIBS -ludev"],
-- 
1.8.1.2

[PATCH 12/12] staging: usbip: userspace: update dependencies in README

[Valentina Manea]

Add libudev as dependency and remove libsysfs.

Signed-off-by: Valentina Manea <valentina.manea.m@gmail.com>
---
 drivers/staging/usbip/userspace/README | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/usbip/userspace/README b/drivers/staging/usbip/userspace/README
index 00a1658..6ead290 100644
--- a/drivers/staging/usbip/userspace/README
+++ b/drivers/staging/usbip/userspace/README
@@ -9,8 +9,8 @@
     - USB/IP device drivers
 	Found in the staging directory of the Linux kernel.
 
-    - sysfsutils >= 2.0.0
-	sysfsutils library
+    - libudev >= 2.0
+	libudev library
 
     - libwrap0-dev
 	tcp wrapper library
-- 
1.8.1.2

Re: [PATCH 00/12] Migrate usbip-utils to libudev

[Greg KH]

On Tue, Mar 04, 2014 at 09:10:40PM +0200, Valentina Manea wrote:
> This patch series modifies the USB/IP userspace side (usbip-utils)
> to use libudev instead of libsysfs. This change was necessary as
> libsysfs is no longer maintained and we have discovered a bug that
> affected USB/IP.

That's great, I didn't even know anything even used libsysfs anymore.

> On the other hand, libudev is actively maintained and recommended
> for interacting with sysfs.

Yes, that's the correct thing to do here, nice job.

greg k-h