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