From d153ca75d0323eb125e525ebdf4ac5c982941d15 Mon Sep 17 00:00:00 2001 From: Peter Jones 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 --- 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, ®ion, &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