Merge branch 'master' into issue-2044

NEWS.adoc

@@ -114,6 +114,10 @@ as part of https://github.com/networkupstools/nut/issues/1410 solution.
      seems to have confused the MGE SHUT (Serial HID UPS Transfer) driver
      support [#2022]
 
+ - An issue was identified which could cause `libupsclient` parser of device
+   and host names to crash upon bad inputs (e.g. poorly resolved environment
+   variables in scripts). Now it should fail more gracefully [#2052]
+
  - New `configure --enable-inplace-runtime` option should set default values
    for `--sysconfdir`, `--with-user` and `--with-group` options to match an
    existing NUT deployment -- for users who are trying if a custom build
@@ -233,6 +237,11 @@ as part of https://github.com/networkupstools/nut/issues/1410 solution.
    of all USB drivers using these options to include the same description
    [#1766]
 
+ - Added a "busport" USB matching option (if supported by the hardware, OS and
+   libusb on the particular deployment, it should allow to specify physical
+   port numbers on an USB hub, rather than logical "device" enumeration values,
+   and in turn -- this should be less volatile across reboots etc.) [#2043]
+
  - Added an `allow_duplicates` flag for common USB matching options which
    may help monitor several related no-name devices (although without knowing
    reliably which one is which... better than nothing) [#1756]

clients/upsclient.c

@@ -1161,7 +1161,7 @@ int upscli_tryconnect(UPSCONN_t *ups, const char *host, uint16_t port, int flags
 
 	pconf_init(&ups->pc_ctx, NULL);
 
-	ups->host = strdup(host);
+	ups->host = xstrdup(host);
 
 	if (!ups->host) {
 		ups->upserror = UPSCLI_ERR_NOMEM;
@@ -1618,22 +1618,45 @@ int upscli_splitname(const char *buf, char **upsname, char **hostname, uint16_t
 
 	s = strchr(tmp, '@');
 
-	if ((*upsname = strdup(strtok_r(tmp, "@", &last))) == NULL) {
-		fprintf(stderr, "upscli_splitname: strdup failed\n");
+	/* someone passed a "@hostname" string? */
+	if (s == tmp) {
+		fprintf(stderr, "upscli_splitname: got empty upsname string\n");
 		return -1;
 	}
 
+	if ((*upsname = xstrdup(strtok_r(tmp, "@", &last))) == NULL) {
+		fprintf(stderr, "upscli_splitname: xstrdup failed\n");
+		return -1;
+	}
+
+	/* someone passed a "@hostname" string (take two)? */
+	if (!**upsname) {
+		fprintf(stderr, "upscli_splitname: got empty upsname string\n");
+		return -1;
+	}
+
+	/*
+	fprintf(stderr, "upscli_splitname3: got buf='%s', tmp='%s', upsname='%s', possible hostname:port='%s'\n",
+		NUT_STRARG(buf), NUT_STRARG(tmp), NUT_STRARG(*upsname), NUT_STRARG((s ? s+1 : s)));
+	*/
+
 	/* only a upsname is specified, fill in defaults */
 	if (s == NULL) {
-		if ((*hostname = strdup("localhost")) == NULL) {
-			fprintf(stderr, "upscli_splitname: strdup failed\n");
+		if ((*hostname = xstrdup("localhost")) == NULL) {
+			fprintf(stderr, "upscli_splitname: xstrdup failed\n");
 			return -1;
 		}
 
 		*port = PORT;
 		return 0;
 	}
 
+	/* someone passed a "upsname@" string? */
+	if (!(*(s+1))) {
+		fprintf(stderr, "upscli_splitname: got the @ separator and then an empty hostname[:port] string\n");
+		return -1;
+	}
+
 	return upscli_splitaddr(s+1, hostname, port);
 }
 
@@ -1659,8 +1682,8 @@ int upscli_splitaddr(const char *buf, char **hostname, uint16_t *port)
 			return -1;
 		}
 
-		if ((*hostname = strdup(strtok_r(tmp+1, "]", &last))) == NULL) {
-			fprintf(stderr, "upscli_splitaddr: strdup failed\n");
+		if ((*hostname = xstrdup(strtok_r(tmp+1, "]", &last))) == NULL) {
+			fprintf(stderr, "upscli_splitaddr: xstrdup failed\n");
 			return -1;
 		}
 
@@ -1672,8 +1695,8 @@ int upscli_splitaddr(const char *buf, char **hostname, uint16_t *port)
 	} else {
 		s = strchr(tmp, ':');
 
-		if ((*hostname = strdup(strtok_r(tmp, ":", &last))) == NULL) {
-			fprintf(stderr, "upscli_splitaddr: strdup failed\n");
+		if ((*hostname = xstrdup(strtok_r(tmp, ":", &last))) == NULL) {
+			fprintf(stderr, "upscli_splitaddr: xstrdup failed\n");
 			return -1;
 		}
 

common/common.c

@@ -1738,7 +1738,14 @@ void *xrealloc(void *ptr, size_t size)
 
 char *xstrdup(const char *string)
 {
-	char *p = strdup(string);
+	char *p;
+
+	if (string == NULL) {
+		upsdebugx(1, "%s: got null input", __func__);
+		return NULL;
+	}
+
+	p = strdup(string);
 
 	if (p == NULL)
 		fatal_with_errno(EXIT_FAILURE, "%s", oom_msg);

docs/man/nut_usb_addvars.txt

@@ -56,18 +56,30 @@ Examples:
 
 Select a UPS on a specific USB bus or group of buses. The argument is
 a regular expression that must match the bus name where the UPS is
-connected (e.g. `bus="002"` or `bus="00[2-3]"`) as seen in
-`/proc/bus/usb/devices` or *lsusb(8)*; including leading zeroes.
+connected (e.g. `bus="002"` or `bus="00[2-3]"`) as seen on Linux in
+`/sys/bus/usb/devices` or *lsusb(8)*; including leading zeroes.
 
 *device =* 'regex'::
 
 Select a UPS on a specific USB device or group of devices. The argument is
 a regular expression that must match the device name where the UPS is
-connected (e.g. `device="001"` or `device="00[1-2]"`) as seen in
-`/proc/bus/usb/devices` or *lsusb(8)*; including leading zeroes.
-Note that device numbers are not guaranteed by the OS to be stable across
+connected (e.g. `device="001"` or `device="00[1-2]"`) as seen on Linux
+in `/sys/bus/usb/devices` or *lsusb(8)*; including leading zeroes.
++
+NOTE: device numbers are not guaranteed by the OS to be stable across
 re-boots or device re-plugging.
 
+*busport =* 'regex'::
+
+If supported by the hardware, OS and libusb on the particular deployment,
+this option should allow to specify physical port numbers on an USB hub,
+rather than logical `device` enumeration values, and in turn -- this should
+be less volatile across reboots or re-plugging.
++
+NOTE: this option is not practically supported by some NUT builds
+(it should be ignored with a warning then), and not by all systems
+that NUT can run on.
+
 *allow_duplicates*::
 
 If you have several UPS devices which may not be uniquely identified by

docs/nut.dict

@@ -1,4 +1,4 @@
-personal_ws-1.1 en 3224 utf-8
+personal_ws-1.1 en 3225 utf-8
 AAS
 ABI
 ACFAIL
@@ -1641,6 +1641,7 @@ bugfixes
 buildbots
 builddir
 bullseye
+busport
 busybox
 bv
 bypassvolts

drivers/blazer_usb.c

@@ -37,7 +37,7 @@
 #endif
 
 #define DRIVER_NAME	"Megatec/Q1 protocol USB driver"
-#define DRIVER_VERSION	"0.16"
+#define DRIVER_VERSION	"0.17"
 
 /* driver description structure */
 upsdrv_info_t upsdrv_info = {
@@ -597,7 +597,7 @@ void upsdrv_initups(void)
 #ifndef TESTING
 	int	ret, langid;
 	char	tbuf[255]; /* Some devices choke on size > 255 */
-	char	*regex_array[7];
+	char	*regex_array[USBMATCHER_REGEXP_ARRAY_LIMIT];
 	char	*subdrv = getval("subdriver");
 
 	warn_if_bad_usb_port_filename(device_path);
@@ -609,6 +609,13 @@ void upsdrv_initups(void)
 	regex_array[4] = getval("serial");
 	regex_array[5] = getval("bus");
 	regex_array[6] = getval("device");
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+	regex_array[7] = getval("busport");
+# else
+	if (getval("busport")) {
+		upslogx(LOG_WARNING, "\"busport\" is configured for the device, but is not actually handled by current build combination of NUT and libusb (ignored)");
+	}
+# endif
 
 	/* check for language ID workaround (#1) */
 	if (getval("langid_fix")) {
@@ -725,5 +732,8 @@ void upsdrv_cleanup(void)
 	free(usbdevice.Serial);
 	free(usbdevice.Bus);
 	free(usbdevice.Device);
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+	free(usbdevice.BusPort);
+# endif
 #endif	/* TESTING */
 }

drivers/libshut.c

@@ -43,7 +43,7 @@
 #include "common.h" /* for xmalloc, upsdebugx prototypes */
 
 #define SHUT_DRIVER_NAME	"SHUT communication driver"
-#define SHUT_DRIVER_VERSION	"0.87"
+#define SHUT_DRIVER_VERSION	"0.88"
 
 /* communication driver description structure */
 upsdrv_info_t comm_upsdrv_info = {
@@ -468,21 +468,36 @@ static int libshut_open(
 	free(curDevice->Serial);
 	free(curDevice->Bus);
 	free(curDevice->Device);
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+	free(curDevice->BusPort);
+#endif
 	memset(curDevice, '\0', sizeof(*curDevice));
 
 	curDevice->VendorID = dev_descriptor->idVendor;
 	curDevice->ProductID = dev_descriptor->idProduct;
 	curDevice->Bus = strdup("serial");
 	curDevice->Device = strdup(arg_device_path);
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+	curDevice->BusPort = (char *)malloc(4);
+	if (curDevice->BusPort == NULL) {
+		fatal_with_errno(EXIT_FAILURE, "Out of memory");
+	}
+	upsdebugx(2, "%s: NOTE: BusPort is always zero with libshut", __func__);
+	sprintf(curDevice->BusPort, "%03d", 0);
+#endif
+
 	curDevice->bcdDevice = dev_descriptor->bcdDevice;
-	curDevice->Vendor = strdup("Eaton");
+	curDevice->Vendor = NULL;
 	if (dev_descriptor->iManufacturer) {
 		ret = shut_get_string_simple(*arg_upsfd, dev_descriptor->iManufacturer,
 			string, MAX_STRING_SIZE);
 		if (ret > 0) {
 			curDevice->Vendor = strdup(string);
 		}
 	}
+	if (curDevice->Vendor == NULL) {
+		curDevice->Vendor = strdup("Eaton");
+	}
 
 	/* ensure iProduct retrieval */
 	if (dev_descriptor->iProduct) {
@@ -514,6 +529,9 @@ static int libshut_open(
 	upsdebugx(2, "- Product: %s", curDevice->Product);
 	upsdebugx(2, "- Serial Number: %s", curDevice->Serial);
 	upsdebugx(2, "- Bus: %s", curDevice->Bus);
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+	upsdebugx(2, "- Bus Port: %s", curDevice->BusPort ? curDevice->BusPort : "unknown");
+#endif
 	upsdebugx(2, "- Device: %s", curDevice->Device ? curDevice->Device : "unknown");
 	upsdebugx(2, "- Device release number: %04x", curDevice->bcdDevice);
 	upsdebugx(2, "Device matches");

drivers/libshut.h

@@ -116,7 +116,7 @@ typedef int usb_ctrl_timeout_msec;	/* in milliseconds */
 
 /*!
  * SHUTDevice_t: Describe a SHUT device. This structure contains exactly
- * the 5 pieces of information by which a SHUT device identifies
+ * the 5 or more pieces of information by which a SHUT device identifies
  * itself, so it serves as a kind of "fingerprint" of the device. This
  * information must be matched exactly when reopening a device, and
  * therefore must not be "improved" or updated by a client
@@ -132,6 +132,9 @@ typedef struct SHUTDevice_s {
 	char*		Bus;       /*!< Bus name, e.g. "003"  */
 	uint16_t	bcdDevice; /*!< Device release number */
 	char		*Device;   /*!< Device name on the bus, e.g. "001"  */
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+	char		*BusPort;  /*!< Port name, e.g. "001"  */
+#endif
 } SHUTDevice_t;
 
 /*!

drivers/libusb0.c

@@ -37,7 +37,7 @@
 #endif
 
 #define USB_DRIVER_NAME		"USB communication driver (libusb 0.1)"
-#define USB_DRIVER_VERSION	"0.44"
+#define USB_DRIVER_VERSION	"0.45"
 
 /* driver description structure */
 upsdrv_info_t comm_upsdrv_info = {
@@ -76,6 +76,11 @@ void nut_usb_addvars(void)
 
 	addvar(VAR_VALUE, "bus", "Regular expression to match USB bus name");
 	addvar(VAR_VALUE, "device", "Regular expression to match USB device name");
+	/* Not supported by libusb0, but let's not crash config
+	 * parsing on unknown keywords due to such nuances! :) */
+	addvar(VAR_VALUE, "busport", "Regular expression to match USB bus port name"
+		" (tolerated but ignored in this build)"
+	);
 
 	/* Warning: this feature is inherently non-deterministic!
 	 * If you only care to know that at least one of your no-name UPSes is online,
@@ -287,6 +292,9 @@ static int libusb_open(usb_dev_handle **udevp,
 			free(curDevice->Serial);
 			free(curDevice->Bus);
 			free(curDevice->Device);
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+			free(curDevice->BusPort);
+#endif
 			memset(curDevice, '\0', sizeof(*curDevice));
 
 			/* Keep the list of items in sync with those matched by
@@ -298,6 +306,15 @@ static int libusb_open(usb_dev_handle **udevp,
 			curDevice->Device = xstrdup(dev->filename);
 			curDevice->bcdDevice = dev->descriptor.bcdDevice;
 
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+			curDevice->BusPort = (char *)malloc(4);
+			if (curDevice->BusPort == NULL) {
+				fatal_with_errno(EXIT_FAILURE, "Out of memory");
+			}
+			upsdebugx(2, "%s: NOTE: BusPort is always zero with libusb0", __func__);
+			sprintf(curDevice->BusPort, "%03d", 0);
+#endif
+
 			if (dev->descriptor.iManufacturer) {
 				retries = MAX_RETRY;
 				while (retries > 0) {
@@ -347,6 +364,9 @@ static int libusb_open(usb_dev_handle **udevp,
 			upsdebugx(2, "- Serial Number: %s", curDevice->Serial ? curDevice->Serial : "unknown");
 			upsdebugx(2, "- Bus: %s", curDevice->Bus ? curDevice->Bus : "unknown");
 			upsdebugx(2, "- Device: %s", curDevice->Device ? curDevice->Device : "unknown");
+#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT)
+			upsdebugx(2, "- Bus Port: %s", curDevice->BusPort ? curDevice->BusPort : "unknown");
+#endif
 			upsdebugx(2, "- Device release number: %04x", curDevice->bcdDevice);
 
 			/* FIXME: extend to Eaton OEMs (HP, IBM, ...) */