Blob Blame History Raw
From d153ca75d0323eb125e525ebdf4ac5c982941d15 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Tue, 15 Oct 2019 16:26:30 -0400
Subject: [PATCH 56/86] Improve consistency of debug prints

This changes debug prints in a couple of ways:
- always calls the path we're parsing "current" in the output
- always use ' not " for quoting in the debug output, so tools that
  escape strings won't change the lenghts
- everything that parses "current" has a debug print after each parse
  attempt and before returning.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 src/dp-acpi.c         |  8 ++++----
 src/linux-acpi-root.c |  4 ++--
 src/linux-acpi.c      |  6 +++---
 src/linux-ata.c       | 11 +++++++----
 src/linux-i2o.c       | 10 +++++-----
 src/linux-md.c        |  4 ++--
 src/linux-pci-root.c  |  2 +-
 src/linux-pmem.c      |  5 +++--
 src/linux.c           | 27 +++++++++++++++++----------
 9 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/src/dp-acpi.c b/src/dp-acpi.c
index 02ec70eec7a..ae4102cdf56 100644
--- a/src/dp-acpi.c
+++ b/src/dp-acpi.c
@@ -1,6 +1,6 @@
 /*
  * libefivar - library for the manipulation of EFI variables
- * Copyright 2012-2015 Red Hat, Inc.
+ * Copyright 2012-2019 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License as
@@ -52,9 +52,9 @@ _format_acpi_hid_ex(unsigned char *buf, size_t size,
 {
 	ssize_t off = 0;
 
-	debug("hid:0x%08x hidstr:\"%s\"", dp->acpi_hid_ex.hid, hidstr);
-	debug("cid:0x%08x cidstr:\"%s\"", dp->acpi_hid_ex.cid, cidstr);
-	debug("uid:0x%08x uidstr:\"%s\"", dp->acpi_hid_ex.uid, uidstr);
+	debug("hid:0x%08x hidstr:'%s'", dp->acpi_hid_ex.hid, hidstr);
+	debug("cid:0x%08x cidstr:'%s'", dp->acpi_hid_ex.cid, cidstr);
+	debug("uid:0x%08x uidstr:'%s'", dp->acpi_hid_ex.uid, uidstr);
 
 	if (!hidstr && !cidstr && (uidstr || dp->acpi_hid_ex.uid)) {
 		format(buf, size, off, "AcpiExp",
diff --git a/src/linux-acpi-root.c b/src/linux-acpi-root.c
index 9154c950bb1..86693c81878 100644
--- a/src/linux-acpi-root.c
+++ b/src/linux-acpi-root.c
@@ -144,7 +144,7 @@ parse_acpi_root(struct device *dev, const char *path, const char *root UNUSED)
 			return rc;
 		}
 	}
-	debug("Parsed HID:0x%08x UID:0x%"PRIx64" uidstr:\"%s\" path:\"%s\"",
+	debug("Parsed HID:0x%08x UID:0x%"PRIx64" uidstr:'%s' path:'%s'",
 	      dev->acpi_root.acpi_hid, dev->acpi_root.acpi_uid,
 	      dev->acpi_root.acpi_uid_str,
 	      dev->acpi_root.acpi_cid_str);
@@ -162,7 +162,7 @@ dp_create_acpi_root(struct device *dev,
 	debug("entry buf:%p size:%zd off:%zd", buf, size, off);
 
 	if (dev->acpi_root.acpi_uid_str || dev->acpi_root.acpi_cid_str) {
-		debug("creating acpi_hid_ex dp hid:0x%08x uid:0x%"PRIx64" uidstr:\"%s\" cidstr:\"%s\"",
+		debug("creating acpi_hid_ex dp hid:0x%08x uid:0x%"PRIx64" uidstr:'%s' cidstr:'%s'",
 		      dev->acpi_root.acpi_hid, dev->acpi_root.acpi_uid,
 		      dev->acpi_root.acpi_uid_str, dev->acpi_root.acpi_cid_str);
 		new = efidp_make_acpi_hid_ex(buf + off, size ? size - off : 0,
diff --git a/src/linux-acpi.c b/src/linux-acpi.c
index 919f4654ae3..534573f32c1 100644
--- a/src/linux-acpi.c
+++ b/src/linux-acpi.c
@@ -1,6 +1,6 @@
 /*
  * libefiboot - library for the manipulation of EFI boot variables
- * Copyright 2012-2018 Red Hat, Inc.
+ * Copyright 2012-2019 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License as
@@ -54,7 +54,7 @@ parse_acpi_hid_uid(struct device *dev, const char *fmt, ...)
 		if (l > 1) {
 			fbuf[l-1] = 0;
 			dev->acpi_root.acpi_cid_str = strdup(fbuf);
-			debug("Setting ACPI root path to \"%s\"", fbuf);
+			debug("Setting ACPI root path to '%s'", fbuf);
 		}
 	}
 
@@ -111,7 +111,7 @@ hid_err:
 			}
 		}
 	}
-	debug("acpi root UID:0x%"PRIx64" uidstr:\"%s\"",
+	debug("acpi root UID:0x%"PRIx64" uidstr:'%s'",
 	      dev->acpi_root.acpi_uid, dev->acpi_root.acpi_uid_str);
 
 	errno = 0;
diff --git a/src/linux-ata.c b/src/linux-ata.c
index a05a2feb8b9..a51ed5abbe0 100644
--- a/src/linux-ata.c
+++ b/src/linux-ata.c
@@ -60,6 +60,7 @@ is_pata(struct device *dev)
 static ssize_t
 parse_ata(struct device *dev, const char *path, const char *root UNUSED)
 {
+	const char *current = path;
 	uint32_t scsi_host, scsi_bus, scsi_device, scsi_target;
 	uint64_t scsi_lun;
 	int pos;
@@ -118,6 +119,7 @@ parse_ata(struct device *dev, const char *path, const char *root UNUSED)
 			      NULL, NULL, NULL);
 	if (pos < 0)
 		return -1;
+	current += pos;
 
 	dev->ata_info.scsi_host = scsi_host;
 	dev->ata_info.scsi_bus = scsi_bus;
@@ -125,10 +127,11 @@ parse_ata(struct device *dev, const char *path, const char *root UNUSED)
 	dev->ata_info.scsi_target = scsi_target;
 	dev->ata_info.scsi_lun = scsi_lun;
 
-	char *block = strstr(path, "/block/");
-	if (!block)
-		return -1;
-	return block + 1 - path;
+	char *block = strstr(current, "/block/");
+	if (block)
+		current += block + 1 - current;
+	debug("current:'%s' sz:%zd", current, current - path);
+	return current - path;
 }
 
 static ssize_t
diff --git a/src/linux-i2o.c b/src/linux-i2o.c
index ebd92aeeb53..b53f4516309 100644
--- a/src/linux-i2o.c
+++ b/src/linux-i2o.c
@@ -1,6 +1,6 @@
 /*
  * libefiboot - library for the manipulation of EFI boot variables
- * Copyright 2012-2018 Red Hat, Inc.
+ * Copyright 2012-2019 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License as
@@ -33,7 +33,7 @@
  * ... probably doesn't work.
  */
 static ssize_t
-parse_i2o(struct device *dev, const char *current UNUSED, const char *root UNUSED)
+parse_i2o(struct device *dev, const char *current, const char *root UNUSED)
 {
 	debug("entry");
 	/* I2O disks can have up to 16 partitions, or 4 bits worth. */
@@ -47,9 +47,9 @@ parse_i2o(struct device *dev, const char *current UNUSED, const char *root UNUSE
 	}
 
 	char *block = strstr(current, "/block/");
-	if (!block)
-	        return -1;
-	return block + 1 - current;
+	ssize_t sz = block ? block + 1 - current : -1;
+	debug("current:'%s' sz:%zd", current, sz);
+	return sz;
 }
 
 enum interface_type i2o_iftypes[] = { i2o, unknown };
diff --git a/src/linux-md.c b/src/linux-md.c
index b0809f8295a..2aacb87469a 100644
--- a/src/linux-md.c
+++ b/src/linux-md.c
@@ -50,7 +50,7 @@ parse_md(struct device *dev, const char *current, const char *root UNUSED)
 	debug("searching for mdM/mdMpN");
 	rc = sscanf(current, "md%d/%nmd%dp%d%n",
 	            &md, &pos0, &tosser0, &part, &pos1);
-	debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1);
+	debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1);
 	dbgmk("         ", pos0, pos1);
 	/*
 	 * If it isn't of that form, it's not one of our partitioned md devices.
@@ -63,10 +63,10 @@ parse_md(struct device *dev, const char *current, const char *root UNUSED)
 	if (dev->part == -1)
 	        dev->part = part;
 
+	debug("current:'%s' sz:%d\n", current, pos1);
 	return pos1;
 }
 
-
 static char *
 make_part_name(struct device *dev)
 {
diff --git a/src/linux-pci-root.c b/src/linux-pci-root.c
index 8cd3849b1db..84189ffbaca 100644
--- a/src/linux-pci-root.c
+++ b/src/linux-pci-root.c
@@ -87,7 +87,7 @@ dp_create_pci_root(struct device *dev UNUSED,
 	debug("entry buf:%p size:%zd off:%zd", buf, size, off);
 	debug("returning 0");
 	if (dev->acpi_root.acpi_uid_str) {
-	        debug("creating acpi_hid_ex dp hid:0x%08x uid:\"%s\"",
+	        debug("creating acpi_hid_ex dp hid:0x%08x uid:'%s'",
 	              dev->acpi_root.acpi_hid,
 	              dev->acpi_root.acpi_uid_str);
 	        new = efidp_make_acpi_hid_ex(buf + off, size ? size - off : 0,
diff --git a/src/linux-pmem.c b/src/linux-pmem.c
index e5de3264d03..b9f83e12e7b 100644
--- a/src/linux-pmem.c
+++ b/src/linux-pmem.c
@@ -103,11 +103,12 @@ parse_pmem(struct device *dev, const char *path, const char *root UNUSED)
 	 *
 	 * 259:0 -> ../../devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region12/btt12.1/block/pmem12s
 	 */
+	pos0 = pos1 = -1;
 	rc = sscanf(current,
 	            "../../devices/%nLNXSYSTM:%hhx/LNXSYBUS:%hhx/ACPI%hx:%hhx/ndbus%d/region%d/btt%d.%d/%n",
 	            &pos0, &system, &sysbus, &pnp_id, &acpi_id, &ndbus,
 		    &region, &btt_region_id, &btt_id, &pos1);
-	debug("current:\"%s\" rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1);
+	debug("current:'%s' rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1);
 	dbgmk("         ", pos0, pos1);
 	if (rc < 8)
 	        return 0;
@@ -126,7 +127,7 @@ parse_pmem(struct device *dev, const char *path, const char *root UNUSED)
 	        return -1;
 
 	filebuf = NULL;
-	debug("nvdimm namespace is \"%s\"", namespace);
+	debug("nvdimm namespace is '%s'", namespace);
 	rc = read_sysfs_file(&filebuf, "bus/nd/devices/%s/uuid", namespace);
 	free(namespace);
 	if (rc < 0 || filebuf == NULL)
diff --git a/src/linux.c b/src/linux.c
index 1f37933f546..1051894e7bc 100644
--- a/src/linux.c
+++ b/src/linux.c
@@ -184,10 +184,10 @@ set_disk_and_part_name(struct device *dev)
 	errno = 0;
 	debug("dev->disk_name:%p dev->part_name:%p", dev->disk_name, dev->part_name);
 	debug("dev->part:%d", dev->part);
-	debug("ultimate:\"%s\"", ultimate ? : "");
-	debug("penultimate:\"%s\"", penultimate ? : "");
-	debug("approximate:\"%s\"", approximate ? : "");
-	debug("proximate:\"%s\"", proximate ? : "");
+	debug("ultimate:'%s'", ultimate ? : "");
+	debug("penultimate:'%s'", penultimate ? : "");
+	debug("approximate:'%s'", approximate ? : "");
+	debug("proximate:'%s'", proximate ? : "");
 
 	if (ultimate && penultimate &&
 	    ((proximate && !strcmp(proximate, "nvme")) ||
@@ -463,7 +463,11 @@ struct device HIDDEN
 	                efi_error("parsing %s failed", probe->name);
 	                goto err;
 	        } else if (pos > 0) {
-	                debug("%s matched %s", probe->name, current);
+			char match[pos+1];
+
+			strncpy(match, current, pos);
+			match[pos] = '\0';
+	                debug("%s matched '%s'", probe->name, match);
 	                dev->flags |= probe->flags;
 
 	                if (probe->flags & DEV_PROVIDES_HD ||
@@ -473,7 +477,10 @@ struct device HIDDEN
 
 	                dev->probes[n++] = dev_probes[i];
 	                current += pos;
-	                debug("current:%s", current);
+			if (current[0] == '\0')
+				debug("finished");
+			else
+				debug("current:'%s'", current);
 	                last_successful_probe = i;
 
 	                if (!*current || !strncmp(current, "block/", 6))
@@ -482,8 +489,8 @@ struct device HIDDEN
 	                continue;
 	        }
 
-	        debug("dev_probes[i+1]: %p dev->interface_type: %d\n",
-	              dev_probes[i+1], dev->interface_type);
+	        debug("dev_probes[%d]: %p dev->interface_type: %d\n",
+	              i+1, dev_probes[i+1], dev->interface_type);
 	        if (dev_probes[i+1] == NULL && dev->interface_type == unknown) {
 	                pos = 0;
 	                rc = sscanf(current, "%*[^/]/%n", &pos);
@@ -499,8 +506,8 @@ slash_err:
 	                if (!current[pos])
 	                        goto slash_err;
 
-	                debug("Cannot parse device link segment \"%s\"", current);
-	                debug("Skipping to \"%s\"", current + pos);
+	                debug("Cannot parse device link segment '%s'", current);
+	                debug("Skipping to '%s'", current + pos);
 	                debug("This means we can only create abbreviated paths");
 	                dev->flags |= DEV_ABBREV_ONLY;
 	                i = last_successful_probe;
-- 
2.24.1