Re: CVS commit: src/sys/dev/pci

●To: Christos Zoulas <christos%astron.com@localhost> ●Subject: Re: CVS commit: src/sys/dev/pci ●From: Paul Goyette <paul%whooppee.com@localhost> ●Date: Fri, 28 Oct 2016 13:18:11 +0800 (PHT)

... 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, ")");
  }
 }

●Follow-Ups: ●Re: CVS commit: src/sys/dev/pci ●From: Christos Zoulas ●Re: CVS commit: src/sys/dev/pci ●From: Paul Goyette ●References: ●re: CVS commit: src/sys/dev/pci ●From: matthew green ●re: CVS commit: src/sys/dev/pci ●From: Paul Goyette ●Re: CVS commit: src/sys/dev/pci ●From: Christos Zoulas ●Re: CVS commit: src/sys/dev/pci ●From: Paul Goyette

●Prev by Date: Re: CVS commit: src/sys/dev/pci ●Next by Date: Re: CVS commit: src/sys/dev/pci ●Previous by Thread: Re: CVS commit: src/sys/dev/pci ●Next by Thread: Re: CVS commit: src/sys/dev/pci ●Indexes: ● reverse Date ● reverse Thread ● Old Index Home | Main Index | Thread Index | Old Index