Blob Blame Raw
From eb7db33c6cf4172551fe0f9f7cf4aa047dc16d88 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Wed, 20 Jun 2018 14:27:11 -0400
Subject: [PATCH 11/17] Improve ACPI device path formatting

This factors a bunch of the duplication out to another function, which
also does a better job of it.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 src/dp-acpi.c                  | 296 ++++++++++++++++++---------------
 src/include/efivar/efivar-dp.h |   2 +-
 2 files changed, 159 insertions(+), 139 deletions(-)

diff --git a/src/dp-acpi.c b/src/dp-acpi.c
index 70162f320dc..a49ef38488c 100644
--- a/src/dp-acpi.c
+++ b/src/dp-acpi.c
@@ -44,6 +44,59 @@ _format_acpi_adr(char *buf, size_t size,
 #define format_acpi_adr(buf, size, off, dp)				\
 	format_helper(_format_acpi_adr, buf, size, off, "AcpiAdr", dp)
 
+static ssize_t
+_format_acpi_hid_ex(char *buf, size_t size, const char *dp_type UNUSED,
+		    const_efidp dp,
+		    const char *hidstr, const char *cidstr, const char *uidstr)
+{
+	ssize_t off = 0;
+
+	debug(DEBUG, "hid:0x%08x hidstr:\"%s\"", dp->acpi_hid_ex.hid, hidstr);
+	debug(DEBUG, "cid:0x%08x cidstr:\"%s\"", dp->acpi_hid_ex.cid, cidstr);
+	debug(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",
+		       "AcpiExp(0x%"PRIx32",0x%"PRIx32",",
+		       dp->acpi_hid_ex.hid, dp->acpi_hid_ex.cid);
+		if (uidstr) {
+			format(buf, size, off, "AcpiExp", "%s)", uidstr);
+		} else {
+			format(buf, size, off, "AcpiExp", "0x%"PRIx32")",
+			       dp->acpi_hid_ex.uid);
+		}
+		return off;
+	}
+
+	format(buf, size, off, "AcpiEx", "AcpiEx(");
+	if (hidstr) {
+		format(buf, size, off, "AcpiEx", "%s,", hidstr);
+	} else {
+		format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
+		       dp->acpi_hid_ex.hid);
+	}
+
+	if (cidstr) {
+		format(buf, size, off, "AcpiEx", "%s,", cidstr);
+	} else {
+		format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
+		       dp->acpi_hid_ex.cid);
+	}
+
+	if (uidstr) {
+		format(buf, size, off, "AcpiEx", "%s)", uidstr);
+	} else {
+		format(buf, size, off, "AcpiEx", "0x%"PRIx32")",
+		       dp->acpi_hid_ex.uid);
+	}
+
+	return off;
+}
+
+#define format_acpi_hid_ex(buf, size, off, dp, hidstr, cidstr, uidstr)  \
+	format_helper(_format_acpi_hid_ex, buf, size, off, "AcpiEx", dp,\
+		      hidstr, cidstr, uidstr)
+
 ssize_t
 _format_acpi_dn(char *buf, size_t size, const_efidp dp)
 {
@@ -53,13 +106,15 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
 	const char *uidstr = NULL;
 	size_t uidlen = 0;
 	const char *cidstr = NULL;
-	size_t cidlen = 0;
+	// size_t cidlen = 0;
 
 	if (dp->subtype == EFIDP_ACPI_ADR) {
+		debug(DEBUG, "formatting ACPI _ADR");
 		format_acpi_adr(buf, size, off, dp);
 		return off;
 	} else if (dp->subtype != EFIDP_ACPI_HID_EX &&
 		   dp->subtype != EFIDP_ACPI_HID) {
+		debug(DEBUG, "DP subtype %d, formatting as ACPI Path", dp->subtype);
 		format(buf, size, off, "AcpiPath", "AcpiPath(%d,", dp->subtype);
 		format_hex(buf, size, off, "AcpiPath", (uint8_t *)dp+4,
 			   (efidp_node_size(dp)-4) / 2);
@@ -69,6 +124,7 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
 		ssize_t limit = efidp_node_size(dp)
 				- offsetof(efidp_acpi_hid_ex, hidstr);
 
+		debug(DEBUG, "formatting ACPI HID EX");
 		hidstr = dp->acpi_hid_ex.hidstr;
 		hidlen = strnlen(hidstr, limit);
 		limit -= hidlen + 1;
@@ -81,7 +137,7 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
 
 		if (limit) {
 			cidstr = uidstr + uidlen + 1;
-			cidlen = strnlen(cidstr, limit);
+			// cidlen = strnlen(cidstr, limit);
 			// limit -= cidlen + 1;
 		}
 
@@ -96,143 +152,102 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
 				       "PcieRoot(%s)", uidstr);
 				return off;
 			default:
-				format(buf, size, off, "AcpiEx", "AcpiEx(");
-				if (hidlen)
-					format(buf, size, off, "AcpiEx", "%s",
-							hidstr);
-				else
-					format(buf, size, off, "AcpiEx", "0x%"PRIx32,
-							dp->acpi_hid_ex.hid);
-				if (cidlen)
-					format(buf, size, off, "AcpiEx", ",%s",
-							cidstr);
-				else
-					format(buf, size, off, "AcpiEx", ",0x%"PRIx32,
-							dp->acpi_hid_ex.cid);
-				if (uidlen)
-					format(buf, size, off, "AcpiEx", ",%s",
-							uidstr);
-				else
-					format(buf, size, off, "AcpiEx", ",0x%"PRIx32,
-							dp->acpi_hid_ex.uid);
-				format(buf, size, off, "AcpiEx", ")");
-				break;
+				format_acpi_hid_ex(buf, size, off, dp,
+						   hidstr, cidstr, uidstr);
+				return off;
 			}
 		}
-	}
-
-	switch (dp->acpi_hid.hid) {
-	case EFIDP_ACPI_PCI_ROOT_HID:
-		format(buf, size, off, "PciRoot", "PciRoot(0x%"PRIx32")",
-		       dp->acpi_hid.uid);
-		break;
-	case EFIDP_ACPI_PCIE_ROOT_HID:
-		format(buf, size, off, "PcieRoot", "PcieRoot(0x%"PRIx32")",
-		       dp->acpi_hid.uid);
-		break;
-	case EFIDP_ACPI_FLOPPY_HID:
-		format(buf, size, off, "Floppy", "Floppy(0x%"PRIx32")",
-		       dp->acpi_hid.uid);
-		break;
-	case EFIDP_ACPI_KEYBOARD_HID:
-		format(buf, size, off, "Keyboard", "Keyboard(0x%"PRIx32")",
-		       dp->acpi_hid.uid);
-		break;
-	case EFIDP_ACPI_SERIAL_HID:
-		format(buf, size, off, "Keyboard", "Serial(0x%"PRIx32")",
-		       dp->acpi_hid.uid);
-		break;
-	case EFIDP_ACPI_NVDIMM_HID: {
-		int rc;
-		const_efidp next = NULL;
-		efidp_acpi_adr *adrdp;
-		int end;
-
-		format(buf, size, off, "NvRoot()", "NvRoot()");
-
-		rc = efidp_next_node(dp, &next);
-		if (rc < 0 || !next) {
-			efi_error("could not format DP");
-			return rc;
-		}
+	} else if (dp->subtype == EFIDP_ACPI_HID_EX) {
+		switch (dp->acpi_hid.hid) {
+		case EFIDP_ACPI_PCI_ROOT_HID:
+			format(buf, size, off, "PciRoot",
+			       "PciRoot(0x%"PRIx32")",
+			       dp->acpi_hid.uid);
+			break;
+		case EFIDP_ACPI_PCIE_ROOT_HID:
+			format(buf, size, off, "PcieRoot",
+			       "PcieRoot(0x%"PRIx32")",
+			       dp->acpi_hid.uid);
+			break;
+		case EFIDP_ACPI_FLOPPY_HID:
+			format(buf, size, off, "Floppy",
+			       "Floppy(0x%"PRIx32")",
+			       dp->acpi_hid.uid);
+			break;
+		case EFIDP_ACPI_KEYBOARD_HID:
+			format(buf, size, off, "Keyboard",
+			       "Keyboard(0x%"PRIx32")",
+			       dp->acpi_hid.uid);
+			break;
+		case EFIDP_ACPI_SERIAL_HID:
+			format(buf, size, off, "Serial",
+			       "Serial(0x%"PRIx32")",
+			       dp->acpi_hid.uid);
+			break;
+		case EFIDP_ACPI_NVDIMM_HID: {
+			int rc;
+			const_efidp next = NULL;
+			efidp_acpi_adr *adrdp;
+			int end;
 
-		if (efidp_type(next) != EFIDP_ACPI_TYPE ||
-		    efidp_subtype(next) != EFIDP_ACPI_ADR) {
-			efi_error("Invalid child node type (0x%02x,0x%02x)",
-				  efidp_type(next), efidp_subtype(next));
-			return -EINVAL;
-		}
-		adrdp = (efidp_acpi_adr *)next;
+			format(buf, size, off, "NvRoot()", "NvRoot()");
 
-		end = efidp_size_after(adrdp, header)
-			/ sizeof(adrdp->adr[0]);
+			rc = efidp_next_node(dp, &next);
+			if (rc < 0 || !next) {
+				efi_error("could not format DP");
+				return rc;
+			}
 
-		for (int i = 0; i < end; i++) {
-			uint32_t node_controller, socket, memory_controller;
-			uint32_t memory_channel, dimm;
-			uint32_t adr = adrdp->adr[i];
+			if (efidp_type(next) != EFIDP_ACPI_TYPE ||
+			    efidp_subtype(next) != EFIDP_ACPI_ADR) {
+				efi_error("Invalid child node type (0x%02x,0x%02x)",
+					  efidp_type(next), efidp_subtype(next));
+				return -EINVAL;
+			}
+			adrdp = (efidp_acpi_adr *)next;
 
-			efidp_decode_acpi_nvdimm_adr(adr, &node_controller,
-						     &socket,
-						     &memory_controller,
-						     &memory_channel, &dimm);
+			end = efidp_size_after(adrdp, header)
+				/ sizeof(adrdp->adr[0]);
 
-			if (i != 0)
-				format(buf, size, off, "NvDimm", ",");
+			for (int i = 0; i < end; i++) {
+				uint32_t node_controller, socket, memory_controller;
+				uint32_t memory_channel, dimm;
+				uint32_t adr = adrdp->adr[i];
 
-			format(buf, size, off, "NvDimm",
-			       "NvDimm(0x%03x,0x%01x,0x%01x,0x%01x,0x%01x)",
-			       node_controller, socket, memory_controller,
-			       memory_channel, dimm);
-		}
-		break;
-				    }
-	default:
-		switch (dp->subtype) {
-		case EFIDP_ACPI_HID_EX:
-			if (!hidstr && !cidstr &&
-					(uidstr || dp->acpi_hid_ex.uid)){
-				format(buf, size, off, "AcpiExp",
-				       "AcpiExp(0x%"PRIx32",0x%"PRIx32",",
-				       dp->acpi_hid_ex.hid,
-				       dp->acpi_hid_ex.cid);
-				if (uidstr) {
-					format(buf, size, off, "AcpiExp",
-					       "%s)", uidstr);
-				} else {
-					format(buf, size, off, "AcpiExp",
-					       "0x%"PRIx32")",
-					       dp->acpi_hid.uid);
-				}
-				break;
-			}
-			format(buf, size, off, "AcpiEx", "AcpiEx(");
-			if (hidstr) {
-				format(buf, size, off, "AcpiEx", "%s,", hidstr);
-			} else {
-				format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
-					      dp->acpi_hid.hid);
-			}
+				efidp_decode_acpi_nvdimm_adr(adr,
+					&node_controller, &socket,
+					&memory_controller, &memory_channel,
+					&dimm);
 
-			if (cidstr) {
-				format(buf, size, off, "AcpiEx", "%s,", cidstr);
-			} else {
-				format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
-				       dp->acpi_hid_ex.cid);
-			}
+				if (i != 0)
+					format(buf, size, off, "NvDimm", ",");
 
-			if (uidstr) {
-				format(buf, size, off, "AcpiEx", "%s)", uidstr);
-			} else {
-				format(buf, size, off, "AcpiEx", "0x%"PRIx32")",
-				       dp->acpi_hid.uid);
+				format(buf, size, off, "NvDimm",
+				       "NvDimm(0x%03x,0x%01x,0x%01x,0x%01x,0x%01x)",
+				       node_controller, socket, memory_controller,
+				       memory_channel, dimm);
 			}
 			break;
-		case EFIDP_ACPI_HID:
-			format(buf, size, off, "Acpi",
-			       "Acpi(0x%"PRIx32",0x%"PRIx32")",
-			       dp->acpi_hid.hid, dp->acpi_hid.uid);
-			break;
+					    }
+		default:
+			debug(DEBUG, "Decoding non-well-known HID");
+			switch (dp->subtype) {
+			case EFIDP_ACPI_HID_EX:
+				format_acpi_hid_ex(buf, size, off, dp,
+						   hidstr, cidstr, uidstr);
+				break;
+			case EFIDP_ACPI_HID:
+				debug(DEBUG, "Decoding ACPI HID");
+				format(buf, size, off, "Acpi",
+				       "Acpi(0x%08x,0x%"PRIx32")",
+				       dp->acpi_hid.hid, dp->acpi_hid.uid);
+				break;
+			default:
+				debug(DEBUG, "ACPI subtype %d???",
+				      dp->subtype);
+				errno = EINVAL;
+				return -1;
+			}
 		}
 	}
 
@@ -259,7 +274,7 @@ efidp_make_acpi_hid(uint8_t *buf, ssize_t size, uint32_t hid, uint32_t uid)
 	return sz;
 }
 
-ssize_t PUBLIC NONNULL(6, 7, 8)
+ssize_t PUBLIC
 efidp_make_acpi_hid_ex(uint8_t *buf, ssize_t size,
 		       uint32_t hid, uint32_t uid, uint32_t cid,
 		       const char *hidstr, const char *uidstr,
@@ -268,21 +283,26 @@ efidp_make_acpi_hid_ex(uint8_t *buf, ssize_t size,
 	efidp_acpi_hid_ex *acpi_hid = (efidp_acpi_hid_ex *)buf;
 	ssize_t req;
 	ssize_t sz;
+	size_t hidlen = hidstr ? strlen(hidstr) : 0;
+	size_t uidlen = uidstr ? strlen(uidstr) : 0;
+	size_t cidlen = cidstr ? strlen(cidstr) : 0;
 
-	req = sizeof (*acpi_hid) + 3 +
-		strlen(hidstr) + strlen(uidstr) + strlen(cidstr);
+	req = sizeof (*acpi_hid) + 3 + hidlen + uidlen + cidlen;
 	sz = efidp_make_generic(buf, size, EFIDP_ACPI_TYPE, EFIDP_ACPI_HID_EX,
 				req);
 	if (size && sz == req) {
-		acpi_hid->uid = uid;
-		acpi_hid->hid = hid;
-		acpi_hid->cid = cid;
+		acpi_hid->hid = hidlen ? 0 : hid;
+		acpi_hid->uid = uidlen ? 0 : uid;
+		acpi_hid->cid = cidlen ? 0 : cid;
 		char *next = (char *)buf+offsetof(efidp_acpi_hid_ex, hidstr);
-		strcpy(next, hidstr);
-		next += strlen(hidstr) + 1;
-		strcpy(next, uidstr);
-		next += strlen(uidstr) + 1;
-		strcpy(next, cidstr);
+		if (hidlen)
+			strcpy(next, hidstr);
+		next += hidlen + 1;
+		if (uidlen)
+			strcpy(next, uidstr);
+		next += uidlen + 1;
+		if (cidlen)
+			strcpy(next, cidstr);
 	}
 
 	if (sz < 0)
diff --git a/src/include/efivar/efivar-dp.h b/src/include/efivar/efivar-dp.h
index 106d3645e21..1b05775ae7e 100644
--- a/src/include/efivar/efivar-dp.h
+++ b/src/include/efivar/efivar-dp.h
@@ -133,7 +133,7 @@ typedef struct {
 	/* three ascii string fields follow */
 	char		hidstr[];
 } EFIVAR_PACKED efidp_acpi_hid_ex;
-extern ssize_t __attribute__((__nonnull__ (6,7,8)))
+extern ssize_t
 efidp_make_acpi_hid_ex(uint8_t *buf, ssize_t size,
                        uint32_t hid, uint32_t uid, uint32_t cid,
                        const char *hidstr, const char *uidstr,
-- 
2.17.1