Merge pull request #603 from bgoglin/generic-infos

generic hwloc_infos structure and API

* store array+count of info attributes in a new hwloc_infos structure, use that everywhere in the API (object info, cpukind info, internally, etc)
 * add hwloc_modify_infos() for a clean/flexible/official way to modify these infos
 * improve tests and hwloc-annotate.

Author: bgoglin

NEWS

@@ -34,6 +34,11 @@ Version 3.0.0
     or "Memory" (for Linux DAX and CXL devices, etc).
   + The "Backend" info attribute of OS devices is now in the topology
     root object together with other backends.
+  + The array and count of info attributes are now embedded in the new
+    struct hwloc_infos_s.
+    - hwloc_infos_s structures may be modified with hwloc_modify_infos().
+    - hwloc_cpukinds_get_info() and hwloc_cpukinds_register() now manipulate
+      infos as such a structure.
 * Tools
   + lstopo has a new --osf option to tune the displaying of object
     attributes and units.

doc/Makefile.am

@@ -349,7 +349,14 @@ man3_object_info_attrsdir = $(man3dir)
 man3_object_info_attrs_DATA = \
         $(DOX_MAN_DIR)/man3/hwlocality_info_attr.3 \
         $(DOX_MAN_DIR)/man3/hwloc_obj_get_info_by_name.3 \
-        $(DOX_MAN_DIR)/man3/hwloc_obj_add_info.3
+        $(DOX_MAN_DIR)/man3/hwloc_get_info_by_name.3 \
+        $(DOX_MAN_DIR)/man3/hwloc_obj_add_info.3 \
+        $(DOX_MAN_DIR)/man3/hwloc_modify_infos.3 \
+        $(DOX_MAN_DIR)/man3/hwloc_modify_infos_op_e.3 \
+        $(DOX_MAN_DIR)/man3/HWLOC_MODIFY_INFOS_OP_ADD.3 \
+        $(DOX_MAN_DIR)/man3/HWLOC_MODIFY_INFOS_OP_ADD_UNIQUE.3 \
+        $(DOX_MAN_DIR)/man3/HWLOC_MODIFY_INFOS_OP_REPLACE.3 \
+        $(DOX_MAN_DIR)/man3/HWLOC_MODIFY_INFOS_OP_REMOVE.3
 
 man3_cpubindingdir = $(man3dir)
 man3_cpubinding_DATA = \

doc/hwloc.doxy

@@ -1970,7 +1970,7 @@ Each object contains a list of such pairs that may be consulted
 manually (looking at the object <tt>infos</tt> array field)
 or using the hwloc_obj_get_info_by_name().
 The user may additionally add new name-value pairs to any object using
-hwloc_obj_add_info() or the \ref cli_hwloc_annotate program.
+hwloc_obj_add_info(), hwloc_modify_infos() or the \ref cli_hwloc_annotate program.
 
 Here is a non-exhaustive list of attributes that may be automatically
 added by hwloc.
@@ -3970,7 +3970,7 @@ and <tt>hwloc_topology_set_userdata_import_callback()</tt>.
 Each object may also contain some <em>info</em> attributes
 (name and value strings) that are setup by hwloc during discovery
 and that may be extended by the user with
-<tt>hwloc_obj_add_info()</tt> (see also \ref attributes).
+<tt>hwloc_obj_add_info()</tt> or <tt>hwloc_modify_infos()</tt> (see also \ref attributes).
 Contrary to the <tt>userdata</tt> field which is unique, multiple info
 attributes may exist for each object, even with the same name.
 These attributes are always exported to XML.

hwloc/cpukinds.c

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2020-2022 Inria.  All rights reserved.
+ * Copyright © 2020-2023 Inria.  All rights reserved.
  * See COPYING in top-level directory.
  */
 
@@ -28,7 +28,7 @@ hwloc_internal_cpukinds_destroy(struct hwloc_topology *topology)
   for(i=0; i<topology->nr_cpukinds; i++) {
     struct hwloc_internal_cpukind_s *kind = &topology->cpukinds[i];
     hwloc_bitmap_free(kind->cpuset);
-    hwloc__free_infos(kind->infos, kind->nr_infos);
+    hwloc__free_infos(&kind->infos);
   }
   free(topology->cpukinds);
   topology->cpukinds = NULL;
@@ -59,8 +59,8 @@ hwloc_internal_cpukinds_dup(hwloc_topology_t new, hwloc_topology_t old)
       goto failed;
     }
     if (hwloc__tma_dup_infos(tma,
-                             &kinds[i].infos, &kinds[i].nr_infos,
-                             old->cpukinds[i].infos, old->cpukinds[i].nr_infos) < 0) {
+                             &kinds[i].infos,
+                             &old->cpukinds[i].infos) < 0) {
       assert(!tma || !tma->dontfree); /* this tma cannot fail to allocate */
       hwloc_bitmap_free(kinds[i].cpuset);
       new->nr_cpukinds = i;
@@ -85,7 +85,7 @@ hwloc_internal_cpukinds_restrict(hwloc_topology_t topology)
     hwloc_bitmap_and(kind->cpuset, kind->cpuset, hwloc_get_root_obj(topology)->cpuset);
     if (hwloc_bitmap_iszero(kind->cpuset)) {
       hwloc_bitmap_free(kind->cpuset);
-      hwloc__free_infos(kind->infos, kind->nr_infos);
+      hwloc__free_infos(&kind->infos);
       memmove(kind, kind+1, (topology->nr_cpukinds - i - 1)*sizeof(*kind));
       i--;
       topology->nr_cpukinds--;
@@ -106,29 +106,29 @@ hwloc__cpukind_check_duplicate_info(struct hwloc_internal_cpukind_s *kind,
                                     const char *name, const char *value)
 {
   unsigned i;
-  for(i=0; i<kind->nr_infos; i++)
-    if (!strcmp(kind->infos[i].name, name)
-        && !strcmp(kind->infos[i].value, value))
+  for(i=0; i<kind->infos.count; i++)
+    if (!strcmp(kind->infos.array[i].name, name)
+        && !strcmp(kind->infos.array[i].value, value))
       return 1;
   return 0;
 }
 
 static __hwloc_inline void
 hwloc__cpukind_add_infos(struct hwloc_internal_cpukind_s *kind,
-                         const struct hwloc_info_s *infos, unsigned nr_infos)
+                         const struct hwloc_infos_s *infos)
 {
   unsigned i;
-  for(i=0; i<nr_infos; i++) {
-    if (hwloc__cpukind_check_duplicate_info(kind, infos[i].name, infos[i].value))
+  for(i=0; i<infos->count; i++) {
+    if (hwloc__cpukind_check_duplicate_info(kind, infos->array[i].name, infos->array[i].value))
       continue;
-    hwloc__add_info(&kind->infos, &kind->nr_infos, infos[i].name, infos[i].value);
+    hwloc__add_info(&kind->infos, infos->array[i].name, infos->array[i].value);
   }
 }
 
 int
 hwloc_internal_cpukinds_register(hwloc_topology_t topology, hwloc_cpuset_t cpuset,
                                  int forced_efficiency,
-                                 const struct hwloc_info_s *infos, unsigned nr_infos,
+                                 const struct hwloc_infos_s *infos,
                                  unsigned long flags)
 {
   struct hwloc_internal_cpukind_s *kinds;
@@ -184,8 +184,9 @@ hwloc_internal_cpukinds_register(hwloc_topology_t topology, hwloc_cpuset_t cpuse
       kinds[newnr].efficiency = HWLOC_CPUKIND_EFFICIENCY_UNKNOWN;
       kinds[newnr].forced_efficiency = forced_efficiency;
       hwloc_bitmap_and(kinds[newnr].cpuset, cpuset, kinds[i].cpuset);
-      hwloc__cpukind_add_infos(&kinds[newnr], kinds[i].infos, kinds[i].nr_infos);
-      hwloc__cpukind_add_infos(&kinds[newnr], infos, nr_infos);
+      hwloc__cpukind_add_infos(&kinds[newnr], &kinds[i].infos);
+      if (infos)
+        hwloc__cpukind_add_infos(&kinds[newnr], infos);
       /* remove cpuset PUs from the existing kind that we just split */
       hwloc_bitmap_andnot(kinds[i].cpuset, kinds[i].cpuset, kinds[newnr].cpuset);
       /* clear cpuset PUs that were taken care of */
@@ -196,7 +197,8 @@ hwloc_internal_cpukinds_register(hwloc_topology_t topology, hwloc_cpuset_t cpuse
     } else if (res == HWLOC_BITMAP_CONTAINS
                || res == HWLOC_BITMAP_EQUAL) {
       /* append new info to existing smaller (or equal) kind */
-      hwloc__cpukind_add_infos(&kinds[i], infos, nr_infos);
+      if (infos)
+        hwloc__cpukind_add_infos(&kinds[i], infos);
       if ((flags & HWLOC_CPUKINDS_REGISTER_FLAG_OVERWRITE_FORCED_EFFICIENCY)
           || kinds[i].forced_efficiency == HWLOC_CPUKIND_EFFICIENCY_UNKNOWN)
         kinds[i].forced_efficiency = forced_efficiency;
@@ -218,7 +220,8 @@ hwloc_internal_cpukinds_register(hwloc_topology_t topology, hwloc_cpuset_t cpuse
     kinds[newnr].cpuset = cpuset;
     kinds[newnr].efficiency = HWLOC_CPUKIND_EFFICIENCY_UNKNOWN;
     kinds[newnr].forced_efficiency = forced_efficiency;
-    hwloc__cpukind_add_infos(&kinds[newnr], infos, nr_infos);
+    if (infos)
+      hwloc__cpukind_add_infos(&kinds[newnr], infos);
     newnr++;
   } else {
     hwloc_bitmap_free(cpuset);
@@ -231,7 +234,7 @@ hwloc_internal_cpukinds_register(hwloc_topology_t topology, hwloc_cpuset_t cpuse
 int
 hwloc_cpukinds_register(hwloc_topology_t topology, hwloc_cpuset_t _cpuset,
                         int forced_efficiency,
-                        unsigned nr_infos, struct hwloc_info_s *infos,
+                        struct hwloc_infos_s *infos,
                         unsigned long flags)
 {
   hwloc_bitmap_t cpuset;
@@ -254,7 +257,7 @@ hwloc_cpukinds_register(hwloc_topology_t topology, hwloc_cpuset_t _cpuset,
   if (forced_efficiency < 0)
     forced_efficiency = HWLOC_CPUKIND_EFFICIENCY_UNKNOWN;
 
-  err = hwloc_internal_cpukinds_register(topology, cpuset, forced_efficiency, infos, nr_infos, HWLOC_CPUKINDS_REGISTER_FLAG_OVERWRITE_FORCED_EFFICIENCY);
+  err = hwloc_internal_cpukinds_register(topology, cpuset, forced_efficiency, infos, HWLOC_CPUKINDS_REGISTER_FLAG_OVERWRITE_FORCED_EFFICIENCY);
   if (err < 0)
     return err;
 
@@ -317,8 +320,8 @@ hwloc__cpukinds_summarize_info(struct hwloc_topology *topology,
 
   for(i=0; i<topology->nr_cpukinds; i++) {
     struct hwloc_internal_cpukind_s *kind = &topology->cpukinds[i];
-    for(j=0; j<kind->nr_infos; j++) {
-      struct hwloc_info_s *info = &kind->infos[j];
+    for(j=0; j<kind->infos.count; j++) {
+      struct hwloc_info_s *info = &kind->infos.array[j];
       if (!strcmp(info->name, "FrequencyMaxMHz")) {
         summary->summaries[i].max_freq = atoi(info->value);
       } else if (!strcmp(info->name, "FrequencyBaseMHz")) {
@@ -598,7 +601,7 @@ hwloc_cpukinds_get_info(hwloc_topology_t topology,
                         unsigned id,
                         hwloc_bitmap_t cpuset,
                         int *efficiencyp,
-                        unsigned *nr_infosp, struct hwloc_info_s **infosp,
+                        struct hwloc_infos_s **infosp,
                         unsigned long flags)
 {
   struct hwloc_internal_cpukind_s *kind;
@@ -621,10 +624,8 @@ hwloc_cpukinds_get_info(hwloc_topology_t topology,
   if (efficiencyp)
     *efficiencyp = kind->efficiency;
 
-  if (nr_infosp && infosp) {
-    *nr_infosp = kind->nr_infos;
-    *infosp = kind->infos;
-  }
+  if (infosp)
+    *infosp = &kind->infos;
   return 0;
 }
 

hwloc/diff.c

@@ -212,10 +212,10 @@ hwloc_diff_trees(hwloc_topology_t topo1, hwloc_obj_t obj1,
 	}
 
 	/* infos */
-	if (obj1->infos_count != obj2->infos_count)
+	if (obj1->infos.count != obj2->infos.count)
 		goto out_too_complex;
-	for(i=0; i<obj1->infos_count; i++) {
-		struct hwloc_info_s *info1 = &obj1->infos[i], *info2 = &obj2->infos[i];
+	for(i=0; i<obj1->infos.count; i++) {
+		struct hwloc_info_s *info1 = &obj1->infos.array[i], *info2 = &obj2->infos.array[i];
 		if (strcmp(info1->name, info2->name))
 			goto out_too_complex;
 		if (strcmp(info1->value, info2->value)) {
@@ -424,10 +424,10 @@ int hwloc_topology_diff_build(hwloc_topology_t topo1,
                 || ic1->efficiency != ic2->efficiency
                 || ic1->forced_efficiency != ic2->forced_efficiency
                 || ic1->ranking_value != ic2->ranking_value
-                || ic1->nr_infos != ic2->nr_infos)
+                || ic1->infos.count != ic2->infos.count)
               goto roottoocomplex;
-            for(j=0; j<ic1->nr_infos; j++) {
-              struct hwloc_info_s *info1 = &ic1->infos[j], *info2 = &ic2->infos[j];
+            for(j=0; j<ic1->infos.count; j++) {
+              struct hwloc_info_s *info1 = &ic1->infos.array[j], *info2 = &ic2->infos.array[j];
               if (strcmp(info1->name, info2->name)
                   || strcmp(info1->value, info2->value)) {
                 goto roottoocomplex;
@@ -494,8 +494,8 @@ hwloc_apply_diff_one(hwloc_topology_t topology,
 			const char *newvalue = reverse ? obj_attr->diff.string.oldvalue : obj_attr->diff.string.newvalue;
 			unsigned i;
 			int found = 0;
-			for(i=0; i<obj->infos_count; i++) {
-				struct hwloc_info_s *info = &obj->infos[i];
+			for(i=0; i<obj->infos.count; i++) {
+				struct hwloc_info_s *info = &obj->infos.array[i];
 				if (!strcmp(info->name, name)
 				    && !strcmp(info->value, oldvalue)) {
 					free(info->value);

hwloc/topology-darwin.c

@@ -320,17 +320,20 @@ static int hwloc__darwin_cpukinds_register(struct hwloc_topology *topology,
   int got_efficiency = kinds->nr ? 1 : 0;
 
   for(i=0; i<kinds->nr; i++) {
+    struct hwloc_infos_s infos;
     struct hwloc_info_s infoattr;
-    unsigned nr_info = 0;
     int efficiency;
     hwloc_debug_2args_bitmap("building cpukind with perflevel %u compatible `%s' and cpuset %s\n",
                              kinds->kinds[i].perflevel,
                              kinds->kinds[i].compatible,
                              kinds->kinds[i].cpuset);
+    infos.count = 0;
     if (kinds->kinds[i].compatible) {
       infoattr.name = (char *) "DarwinCompatible";
       infoattr.value = kinds->kinds[i].compatible;
-      nr_info = 1;
+      infos.array = &infoattr;
+      infos.count = 1;
+      infos.allocated = 0;
     }
     if (kinds->kinds[i].perflevel >= 0) {
       /* perflevel0 always refers to the highest performance core type in the system. */
@@ -339,7 +342,7 @@ static int hwloc__darwin_cpukinds_register(struct hwloc_topology *topology,
       efficiency = HWLOC_CPUKIND_EFFICIENCY_UNKNOWN;
       got_efficiency = 0;
     }
-    hwloc_internal_cpukinds_register(topology, kinds->kinds[i].cpuset, efficiency, &infoattr, nr_info, 0);
+    hwloc_internal_cpukinds_register(topology, kinds->kinds[i].cpuset, efficiency, &infos, 0);
     free(kinds->kinds[i].compatible);
     /* the cpuset is given to the callee */
   }