diff mbox series

[v3] tunables: sort tunables list (BZ 30027)

Message ID xnr0d2t21o.fsf@greed.delorie.com
State New
Headers show
Series [v3] tunables: sort tunables list (BZ 30027) | expand

Commit Message

DJ Delorie June 12, 2024, 7:05 p.m. UTC 
[v3: added commit message and LC_ALL=C]

Sort tunables list at the time it's generated.  Note: adding new
tunables will cause other tunable IDs to change, but that was
the case before anyway.  POSIX does not guarantee the order of "foo
in bar" AWK operators, so the order was indeterminate before anyway.
Even depending on the order to be the same across multiple calls,
such as in this script, is undefined, so sorting the list resolves
that also.

Note that sorting is not dependent on the user's locale.

Comments

Florian Weimer June 12, 2024, 7:12 p.m. UTC | #1 
* DJ Delorie:

> diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
> index fc3b41376f..d9f326b63b 100644
> --- a/scripts/gen-tunables.awk
> +++ b/scripts/gen-tunables.awk
> @@ -131,6 +131,11 @@ END {
>      exit 1
>    }
>  
> +  # TYPES is an associative array where the index is the data
> +  # TYPESA is an indexed array where the value is the data
> +  # We sort TYPESA
> +  typecount = asorti (types, typesa)
> +
>    print "/* AUTOGENERATED by gen-tunables.awk.  */"
>    print "#ifndef _TUNABLES_H_"
>    print "# error \"Do not include this file directly.\""
> @@ -141,7 +146,8 @@ END {
>    # Now, the enum names
>    print "\ntypedef enum"
>    print "{"
> -  for (tnm in types) {
> +  for (i = 1; i <= typecount; i++) {
> +    tnm = typesa[i];

Sorry for the piecewise review, but there seems to be a types variable
reference that the patch doesn't update:

    printf (", {TUNABLE_TYPE_%s, %s, %s}, {%s}, {%s}, false, %s},\n",
	    types[t,n,m], minvals[t,n,m], maxvals[t,n,m], default_val[t,n,m],
	    default_val[t,n,m], env_alias[t,n,m]);

As the original keys stored in types are destroyed, types[t,n,m]
shouldn't work anymore, so I'm not sure how this passed testing.  I
think the correct replacement would be types[i], but I haven't tested
this.

Thanks,
Florian
diff mbox series

Patch

diff --git a/Makeconfig b/Makeconfig
index 29819363da..2d4343b604 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -1265,7 +1265,7 @@  $(common-objpfx)dl-tunable-list.stmp: \
 		$(..)elf/dl-tunables.list \
 		$(wildcard $(subdirs:%=$(..)%/dl-tunables.list)) \
 		$(wildcard $(sysdirs:%=%/dl-tunables.list))
-	$(AWK) -f $^ > ${@:stmp=T}
+	LC_ALL=C $(AWK) -f $^ > ${@:stmp=T}
 	$(move-if-change) ${@:stmp=T} ${@:stmp=h}
 	touch $@
 
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index fc3b41376f..d9f326b63b 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -131,6 +131,11 @@  END {
     exit 1
   }
 
+  # TYPES is an associative array where the index is the data
+  # TYPESA is an indexed array where the value is the data
+  # We sort TYPESA
+  typecount = asorti (types, typesa)
+
   print "/* AUTOGENERATED by gen-tunables.awk.  */"
   print "#ifndef _TUNABLES_H_"
   print "# error \"Do not include this file directly.\""
@@ -141,7 +146,8 @@  END {
   # Now, the enum names
   print "\ntypedef enum"
   print "{"
-  for (tnm in types) {
+  for (i = 1; i <= typecount; i++) {
+    tnm = typesa[i];
     split (tnm, indices, SUBSEP);
     t = indices[1];
     n = indices[2];
@@ -157,7 +163,8 @@  END {
   print "# include \"dl-tunable-types.h\""
   # Finally, the tunable list.
   print "static tunable_t tunable_list[] attribute_relro __attribute_used__ = {"
-  for (tnm in types) {
+  for (i = 1; i <= typecount; i++) {
+    tnm = typesa[i];
     split (tnm, indices, SUBSEP);
     t = indices[1];
     n = indices[2];
@@ -172,7 +179,8 @@  END {
   # Map of tunable with environment variables aliases used during parsing.  */
   print "\nstatic const tunable_id_t tunable_env_alias_list[] ="
   printf "{\n"
-  for (tnm in types) {
+  for (i = 1; i <= typecount; i++) {
+    tnm = typesa[i];
     split (tnm, indices, SUBSEP);
     t = indices[1];
     n = indices[2];