... I think that perhaps a variant that returns the
number of characters appended would be more useful. like
if (len >= max)
dest = end;
else
dest += len;
but, I would leave the whole thing alone instead :-)
or write a function that appends and moves the pointer, like
snappendprintf(char **dest, const char *end, fmt, ...)
snappendprintf() seemed a rather unwieldly function name, so I called it
snappendf()!
The attached diffs implement this function and use in throughout
pci_devinfo().
At some time in the future we will eventually get products whose verbose
description exceed the currently-allocated 256-byte buffer, so I really
think we should implement this (or something similar) to protect against
buffer overflows. Does anyone have any major objections? Are there
better alternatives than snappendf() that should be pursued instead?
+------------------+--------------------------+------------------------+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+------------------+--------------------------+------------------------+
Index: pci_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pci_subr.c,v
retrieving revision 1.152
diff -u -p -r1.152 pci_subr.c
--- pci_subr.c 20 Oct 2016 04:11:02 -0000 1.152
+++ pci_subr.c 28 Oct 2016 05:07:41 -0000
@@ -54,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: pci_subr.c,v
#include <sys/module.h>
#else
#include <pci.h>
+#include <stdarg.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
@@ -567,6 +568,25 @@ static const struct pci_class pci_class[
DEV_VERBOSE_DEFINE(pci);
+/*
+ * Append a formatted string into dest without writing beyond end, and
+ * update dest to point to next available character position
+ */
+static void
+snappendf(char **dest, const char *end, const char * restrict fmt, ...)
+{
+ va_list ap;
+ int count;
+ int len = end - *dest;
+
+ va_start(ap, fmt);
+ count = vsnprintf(*dest, len, fmt, ap);
+ va_end(ap);
+ if (count > len)
+ count = len;
+ *dest += count;
+}
+
void
pci_devinfo(pcireg_t id_reg, pcireg_t class_reg, int showclass, char *cp,
size_t l)
@@ -612,32 +632,29 @@ pci_devinfo(pcireg_t id_reg, pcireg_t cl
interfacep++;
}
- cp += snprintf(cp, ep - cp, "%s %s", vendor, product);
+ snappendf(&cp, ep, "%s %s", vendor, product);
if (showclass) {
- cp += snprintf(cp, ep - cp, " (");
+ snappendf(&cp, ep, " (");
if (classp->name == NULL)
- cp += snprintf(cp, ep - cp,
- "class 0x%02x, subclass 0x%02x", pciclass,
subclass);
+ snappendf(&cp, ep, "class 0x%02x, subclass 0x%02x",
+ pciclass, subclass);
else {
if (subclassp == NULL || subclassp->name == NULL)
- cp += snprintf(cp, ep - cp,
- "%s, subclass 0x%02x",
+ snappendf(&cp, ep, "%s, subclass 0x%02x",
classp->name, subclass);
else
- cp += snprintf(cp, ep - cp, "%s %s",
+ snappendf(&cp, ep, "%s %s",
subclassp->name, classp->name);
}
if ((interfacep == NULL) || (interfacep->name == NULL)) {
if (interface != 0)
- cp += snprintf(cp, ep - cp,
- ", interface 0x%02x", interface);
+ snappendf(&cp, ep, ", interface 0x%02x",
+ interface);
} else if (strncmp(interfacep->name, "", 1) != 0)
- cp += snprintf(cp, ep - cp, ", %s",
- interfacep->name);
+ snappendf(&cp, ep, ", %s", interfacep->name);
if (revision != 0)
- cp += snprintf(cp, ep - cp, ", revision 0x%02x",
- revision);
- cp += snprintf(cp, ep - cp, ")");
+ snappendf(&cp, ep, ", revision 0x%02x", revision);
+ snappendf(&cp, ep, ")");
}
}
