Simplified pci_fill_info() and friendsfill-info

Previously, we kept track of which fields were already filled, which
was quite brittle.

Now we keep only the set of already known fields in struct pci_dev.
We check if the current field is needed against this information.

Not only this simplifies the whole thing, but it also enables future
back-ends to call pci_fill_info() recursively as needed.

7 files changed, 66 insertions, 98 deletions

diff --git a/lib/access.c b/lib/access.c
index b257849..9bd6989 100644
--- a/lib/access.c
+++ b/lib/access.c
@@ -1,7 +1,7 @@
 /*
  *	The PCI Library -- User Access
  *
- *	Copyright (c) 1997--2018 Martin Mares <mj@ucw.cz>
+ *	Copyright (c) 1997--2022 Martin Mares <mj@ucw.cz>
  *
  *	Can be freely distributed and used under the terms of the GNU GPL.
  */
@@ -198,7 +198,7 @@ pci_fill_info_v35(struct pci_dev *d, int flags)
       pci_reset_properties(d);
     }
   if (uflags & ~d->known_fields)
-    d->known_fields |= d->methods->fill_info(d, flags & ~d->known_fields);
+    d->methods->fill_info(d, uflags);
   return d->known_fields;
 }
 
diff --git a/lib/caps.c b/lib/caps.c
index c3b9180..3c025a9 100644
--- a/lib/caps.c
+++ b/lib/caps.c
@@ -80,17 +80,16 @@ pci_scan_ext_caps(struct pci_dev *d)
   while (where);
 }
 
-unsigned int
+void
 pci_scan_caps(struct pci_dev *d, unsigned int want_fields)
 {
-  if ((want_fields & PCI_FILL_EXT_CAPS) && !(d->known_fields & PCI_FILL_CAPS))
+  if (want_fields & PCI_FILL_EXT_CAPS)
     want_fields |= PCI_FILL_CAPS;
 
-  if (want_fields & PCI_FILL_CAPS)
+  if (want_fill(d, want_fields, PCI_FILL_CAPS))
     pci_scan_trad_caps(d);
-  if (want_fields & PCI_FILL_EXT_CAPS)
+  if (want_fill(d, want_fields, PCI_FILL_EXT_CAPS))
     pci_scan_ext_caps(d);
-  return want_fields;
 }
 
 void
diff --git a/lib/fbsd-device.c b/lib/fbsd-device.c
index cffab69..396ff1d 100644
--- a/lib/fbsd-device.c
+++ b/lib/fbsd-device.c
@@ -159,7 +159,7 @@ fbsd_scan(struct pci_access *a)
   free(matches);
 }
 
-static int
+static void
 fbsd_fill_info(struct pci_dev *d, int flags)
 {
   struct pci_conf_io conf;
@@ -200,16 +200,14 @@ fbsd_fill_info(struct pci_dev *d, int flags)
       d->access->error("fbsd_fill_info: ioctl(PCIOCGETCONF) failed: %s", strerror(errno));
     }
 
-  if (flags & PCI_FILL_IDENT)
+  if (want_fill(d, flags, PCI_FILL_IDENT))
     {
       d->vendor_id = match.pc_vendor;
       d->device_id = match.pc_device;
     }
-  if (flags & PCI_FILL_CLASS)
-    {
-      d->device_class = (match.pc_class << 8) | match.pc_subclass;
-    }
-  if (flags & (PCI_FILL_BASES | PCI_FILL_SIZES))
+  if (want_fill(d, flags, PCI_FILL_CLASS))
+    d->device_class = (match.pc_class << 8) | match.pc_subclass;
+  if (want_fill(d, flags PCI_FILL_BASES | PCI_FILL_SIZES))
     {
       d->rom_base_addr = 0;
       d->rom_size = 0;
@@ -242,9 +240,6 @@ fbsd_fill_info(struct pci_dev *d, int flags)
 	    }
 	}
     }
-
-  return flags & (PCI_FILL_IDENT | PCI_FILL_CLASS | PCI_FILL_BASES |
-		  PCI_FILL_SIZES);
 }
 
 static int
diff --git a/lib/generic.c b/lib/generic.c
index ef9e2a3..e80158f 100644
--- a/lib/generic.c
+++ b/lib/generic.c
@@ -1,7 +1,7 @@
 /*
  *	The PCI Library -- Generic Direct Access Functions
  *
- *	Copyright (c) 1997--2000 Martin Mares <mj@ucw.cz>
+ *	Copyright (c) 1997--2022 Martin Mares <mj@ucw.cz>
  *
  *	Can be freely distributed and used under the terms of the GNU GPL.
  */
@@ -74,39 +74,36 @@ pci_generic_scan(struct pci_access *a)
   pci_generic_scan_bus(a, busmap, 0);
 }
 
-unsigned int
+static int
+get_hdr_type(struct pci_dev *d)
+{
+  if (d->hdrtype < 0)
+    d->hdrtype = pci_read_byte(d, PCI_HEADER_TYPE) & 0x7f;
+  return d->hdrtype;
+}
+
+void
 pci_generic_fill_info(struct pci_dev *d, unsigned int flags)
 {
   struct pci_access *a = d->access;
-  unsigned int done = 0;
 
-  if ((flags & (PCI_FILL_BASES | PCI_FILL_ROM_BASE)) && d->hdrtype < 0)
-    d->hdrtype = pci_read_byte(d, PCI_HEADER_TYPE) & 0x7f;
-
-  if (flags & PCI_FILL_IDENT)
+  if (want_fill(d, flags, PCI_FILL_IDENT))
     {
       d->vendor_id = pci_read_word(d, PCI_VENDOR_ID);
       d->device_id = pci_read_word(d, PCI_DEVICE_ID);
-      done |= PCI_FILL_IDENT;
     }
 
-  if (flags & PCI_FILL_CLASS)
-    {
-      d->device_class = pci_read_word(d, PCI_CLASS_DEVICE);
-      done |= PCI_FILL_CLASS;
-    }
+  if (want_fill(d, flags, PCI_FILL_CLASS))
+    d->device_class = pci_read_word(d, PCI_CLASS_DEVICE);
 
-  if (flags & PCI_FILL_IRQ)
-    {
-      d->irq = pci_read_byte(d, PCI_INTERRUPT_LINE);
-      done |= PCI_FILL_IRQ;
-    }
+  if (want_fill(d, flags, PCI_FILL_IRQ))
+    d->irq = pci_read_byte(d, PCI_INTERRUPT_LINE);
 
-  if (flags & PCI_FILL_BASES)
+  if (want_fill(d, flags, PCI_FILL_BASES))
     {
       int cnt = 0, i;
       memset(d->base_addr, 0, sizeof(d->base_addr));
-      switch (d->hdrtype)
+      switch (get_hdr_type(d))
 	{
 	case PCI_HEADER_TYPE_NORMAL:
 	  cnt = 6;
@@ -148,14 +145,13 @@ pci_generic_fill_info(struct pci_dev *d, unsigned int flags)
 		}
 	    }
 	}
-      done |= PCI_FILL_BASES;
     }
 
-  if (flags & PCI_FILL_ROM_BASE)
+  if (want_fill(d, flags, PCI_FILL_ROM_BASE))
     {
       int reg = 0;
       d->rom_base_addr = 0;
-      switch (d->hdrtype)
+      switch (get_hdr_type(d))
 	{
 	case PCI_HEADER_TYPE_NORMAL:
 	  reg = PCI_ROM_ADDRESS;
@@ -170,13 +166,9 @@ pci_generic_fill_info(struct pci_dev *d, unsigned int flags)
 	  if (u != 0xffffffff)
 	    d->rom_base_addr = u;
 	}
-      done |= PCI_FILL_ROM_BASE;
     }
 
-  if (flags & (PCI_FILL_CAPS | PCI_FILL_EXT_CAPS))
-    done |= pci_scan_caps(d, flags);
-
-  return done;
+  pci_scan_caps(d, flags);
 }
 
 static int
diff --git a/lib/hurd.c b/lib/hurd.c
index 90cf89f..0939a20 100644
--- a/lib/hurd.c
+++ b/lib/hurd.c
@@ -328,26 +328,16 @@ hurd_fill_rom(struct pci_dev *d)
   d->rom_size = rom.size;
 }
 
-static unsigned int
+static void
 hurd_fill_info(struct pci_dev *d, unsigned int flags)
 {
-  unsigned int done = 0;
-
   if (!d->access->buscentric)
     {
-      if (flags & (PCI_FILL_BASES | PCI_FILL_SIZES))
-	{
-	  hurd_fill_regions(d);
-	  done |= PCI_FILL_BASES | PCI_FILL_SIZES;
-	}
-      if (flags & PCI_FILL_ROM_BASE)
-	{
-	  hurd_fill_rom(d);
-	  done |= PCI_FILL_ROM_BASE;
-	}
+      if (want_fill(d, flags, PCI_FILL_BASES | PCI_FILL_SIZES))
+	hurd_fill_regions(d);
+      if (want_fill(d, flags, PCI_FILL_ROM_BASE))
+	hurd_fill_rom(d);
     }
-
-  return done | pci_generic_fill_info(d, flags & ~done);
 }
 
 struct pci_methods pm_hurd = {
diff --git a/lib/internal.h b/lib/internal.h
index 17c27e1..942e3cb 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -1,7 +1,7 @@
 /*
  *	The PCI Library -- Internal Stuff
  *
- *	Copyright (c) 1997--2018 Martin Mares <mj@ucw.cz>
+ *	Copyright (c) 1997--2022 Martin Mares <mj@ucw.cz>
  *
  *	Can be freely distributed and used under the terms of the GNU GPL.
  */
@@ -41,7 +41,7 @@ struct pci_methods {
   void (*init)(struct pci_access *);
   void (*cleanup)(struct pci_access *);
   void (*scan)(struct pci_access *);
-  unsigned int (*fill_info)(struct pci_dev *, unsigned int flags);
+  void (*fill_info)(struct pci_dev *, unsigned int flags);
   int (*read)(struct pci_dev *, int pos, byte *buf, int len);
   int (*write)(struct pci_dev *, int pos, byte *buf, int len);
   int (*read_vpd)(struct pci_dev *, int pos, byte *buf, int len);
@@ -52,7 +52,7 @@ struct pci_methods {
 /* generic.c */
 void pci_generic_scan_bus(struct pci_access *, byte *busmap, int bus);
 void pci_generic_scan(struct pci_access *);
-unsigned int pci_generic_fill_info(struct pci_dev *, unsigned int flags);
+void pci_generic_fill_info(struct pci_dev *, unsigned int flags);
 int pci_generic_block_read(struct pci_dev *, int pos, byte *buf, int len);
 int pci_generic_block_write(struct pci_dev *, int pos, byte *buf, int len);
 
@@ -75,6 +75,18 @@ int pci_fill_info_v33(struct pci_dev *, int flags) VERSIONED_ABI;
 int pci_fill_info_v34(struct pci_dev *, int flags) VERSIONED_ABI;
 int pci_fill_info_v35(struct pci_dev *, int flags) VERSIONED_ABI;
 
+static inline int want_fill(struct pci_dev *d, unsigned want_fields, unsigned int try_fields)
+{
+  want_fields &= try_fields;
+  if ((d->known_fields & want_fields) == want_fields)
+    return 0;
+  else
+    {
+      d->known_fields |= try_fields;
+      return 1;
+    }
+}
+
 struct pci_property {
   struct pci_property *next;
   u32 key;
@@ -89,7 +101,7 @@ int pci_set_param_internal(struct pci_access *acc, char *param, char *val, int c
 void pci_free_params(struct pci_access *acc);
 
 /* caps.c */
-unsigned int pci_scan_caps(struct pci_dev *, unsigned int want_fields);
+void pci_scan_caps(struct pci_dev *, unsigned int want_fields);
 void pci_free_caps(struct pci_dev *);
 
 extern struct pci_methods pm_intel_conf1, pm_intel_conf2, pm_linux_proc,
diff --git a/lib/sysfs.c b/lib/sysfs.c
index fb64241..4b8b2cd 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -288,11 +288,9 @@ sysfs_fill_slots(struct pci_access *a)
   closedir(dir);
 }
 
-static unsigned int
+static void
 sysfs_fill_info(struct pci_dev *d, unsigned int flags)
 {
-  unsigned int done = 0;
-
   if (!d->access->buscentric)
     {
       /*
@@ -300,61 +298,45 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags)
        *  the kernel's view, which has regions and IRQs remapped and other fields
        *  (most importantly classes) possibly fixed if the device is known broken.
        */
-      if (flags & PCI_FILL_IDENT)
+      if (want_fill(d, flags, PCI_FILL_IDENT))
 	{
 	  d->vendor_id = sysfs_get_value(d, "vendor", 1);
 	  d->device_id = sysfs_get_value(d, "device", 1);
-	  done |= PCI_FILL_IDENT;
 	}
-      if (flags & PCI_FILL_CLASS)
-	{
-	  d->device_class = sysfs_get_value(d, "class", 1) >> 8;
-	  done |= PCI_FILL_CLASS;
-	}
-      if (flags & PCI_FILL_IRQ)
-	{
+      if (want_fill(d, flags, PCI_FILL_CLASS))
+	d->device_class = sysfs_get_value(d, "class", 1) >> 8;
+      if (want_fill(d, flags, PCI_FILL_IRQ))
 	  d->irq = sysfs_get_value(d, "irq", 1);
-	  done |= PCI_FILL_IRQ;
-	}
-      if (flags & (PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS))
-	{
+      if (want_fill(d, flags, PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS))
 	  sysfs_get_resources(d);
-	  done |= PCI_FILL_BASES | PCI_FILL_ROM_BASE | PCI_FILL_SIZES | PCI_FILL_IO_FLAGS;
-	}
     }
 
-  if (flags & PCI_FILL_PHYS_SLOT)
+  if (want_fill(d, flags, PCI_FILL_PHYS_SLOT))
     {
       struct pci_dev *pd;
       sysfs_fill_slots(d->access);
       for (pd = d->access->devices; pd; pd = pd->next)
 	pd->known_fields |= PCI_FILL_PHYS_SLOT;
-      done |= PCI_FILL_PHYS_SLOT;
     }
 
-  if (flags & PCI_FILL_MODULE_ALIAS)
+  if (want_fill(d, flags, PCI_FILL_MODULE_ALIAS))
     {
       char buf[OBJBUFSIZE];
       if (sysfs_get_string(d, "modalias", buf, 0))
 	d->module_alias = pci_set_property(d, PCI_FILL_MODULE_ALIAS, buf);
-      done |= PCI_FILL_MODULE_ALIAS;
     }
 
-  if (flags & PCI_FILL_LABEL)
+  if (want_fill(d, flags, PCI_FILL_LABEL))
     {
       char buf[OBJBUFSIZE];
       if (sysfs_get_string(d, "label", buf, 0))
 	d->label = pci_set_property(d, PCI_FILL_LABEL, buf);
-      done |= PCI_FILL_LABEL;
     }
 
-  if (flags & PCI_FILL_NUMA_NODE)
-    {
-      d->numa_node = sysfs_get_value(d, "numa_node", 0);
-      done |= PCI_FILL_NUMA_NODE;
-    }
+  if (want_fill(d, flags, PCI_FILL_NUMA_NODE))
+    d->numa_node = sysfs_get_value(d, "numa_node", 0);
 
-  if (flags & PCI_FILL_IOMMU_GROUP)
+  if (want_fill(d, flags, PCI_FILL_IOMMU_GROUP))
     {
       char *group_link = sysfs_deref_link(d, "iommu_group");
       if (group_link)
@@ -362,10 +344,9 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags)
           pci_set_property(d, PCI_FILL_IOMMU_GROUP, basename(group_link));
           free(group_link);
         }
-      done |= PCI_FILL_IOMMU_GROUP;
     }
 
-  if (flags & PCI_FILL_DT_NODE)
+  if (want_fill(d, flags, PCI_FILL_DT_NODE))
     {
       char *node = sysfs_deref_link(d, "of_node");
       if (node)
@@ -373,10 +354,9 @@ sysfs_fill_info(struct pci_dev *d, unsigned int flags)
 	  pci_set_property(d, PCI_FILL_DT_NODE, node);
 	  free(node);
 	}
-      done |= PCI_FILL_DT_NODE;
     }
 
-  return done | pci_generic_fill_info(d, flags & ~done);
+  pci_generic_fill_info(d, flags);
 }
 
 /* Intent of the sysfs_setup() caller */