dcavalca / rpms / grub2

Forked from rpms/grub2 3 years ago
Clone

Blame SOURCES/0152-Simplify-BLS-entry-key-val-pairs-lookup.patch

d9d99f
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
d9d99f
From: Javier Martinez Canillas <javierm@redhat.com>
d9d99f
Date: Fri, 11 May 2018 23:47:31 +0200
d9d99f
Subject: [PATCH] Simplify BLS entry key val pairs lookup
d9d99f
d9d99f
The <key,value> pairs found in the BLS are being sorted but this isn't
d9d99f
really needed and it makes the implementation complex and error prone.
d9d99f
d9d99f
For example, the current implementation has the following issues:
d9d99f
d9d99f
1) Fields not present in the grub2 menu entry
d9d99f
d9d99f
  linux /linuz
d9d99f
  initrd /foo
d9d99f
  initrd /bar
d9d99f
d9d99f
  load_video
d9d99f
  set gfx_payload=keep
d9d99f
  insmod gzio
d9d99f
  linux /boot/linuz
d9d99f
  initrd /boot/bar
d9d99f
d9d99f
2) Fields present but in the wrong order
d9d99f
d9d99f
  title Fedora (4.16.6-300.fc28.x86_64-tuned) 28 (Twenty Eight)
d9d99f
  version 4.16.6-300.fc28.x86_64
d9d99f
  linux /vmlinuz-4.16.6-300.fc28.x86_64
d9d99f
  initrd /foo.img
d9d99f
  initrd /bar.img
d9d99f
  options $kernelopts
d9d99f
  id fedora-20180430150025-4.16.6-300.fc28.x86_64
d9d99f
d9d99f
  load_video
d9d99f
  set gfx_payload=keep
d9d99f
  insmod gzio
d9d99f
  linux /boot/vmlinuz-4.16.6-300.fc28.x86_64 $kernelopts
d9d99f
  initrd /boot/bar.img /boot/foo.img
d9d99f
d9d99f
It's important to preserve the order in which fields have been defined
d9d99f
in the BLS fragment since for some of the fields the order has meaning.
d9d99f
For example, initramfs images have to be passed to the kernel in order
d9d99f
that were defined in the BLS fragment.
d9d99f
d9d99f
This patch simplifies the <key,value> pairs storage and lookup. Rather
d9d99f
than sorting and attempt to later figure out what's the expected order,
d9d99f
just store it in the same order as they were defined in the BLS config
d9d99f
file and return in that same order to callers when these look them up.
d9d99f
d9d99f
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
d9d99f
---
d9d99f
 grub-core/commands/blscfg.c | 88 ++++++++++-----------------------------------
d9d99f
 1 file changed, 18 insertions(+), 70 deletions(-)
d9d99f
d9d99f
diff --git a/grub-core/commands/blscfg.c b/grub-core/commands/blscfg.c
d9d99f
index c52d2b2e05a..fb08d8e4c12 100644
d9d99f
--- a/grub-core/commands/blscfg.c
d9d99f
+++ b/grub-core/commands/blscfg.c
d9d99f
@@ -169,84 +169,35 @@ static void bls_free_entry(struct bls_entry *entry)
d9d99f
   grub_free (entry);
d9d99f
 }
d9d99f
 
d9d99f
-static int keyval_cmp (const void *p0, const void *p1,
d9d99f
-		       void *state UNUSED)
d9d99f
-{
d9d99f
-  const struct keyval *kv0 = *(struct keyval * const *)p0;
d9d99f
-  const struct keyval *kv1 = *(struct keyval * const *)p1;
d9d99f
-  int rc;
d9d99f
-
d9d99f
-  rc = grub_strcmp(kv0->key, kv1->key);
d9d99f
-
d9d99f
-  return rc;
d9d99f
-}
d9d99f
-
d9d99f
 /* Find they value of the key named by keyname.  If there are allowed to be
d9d99f
  * more than one, pass a pointer to an int set to -1 the first time, and pass
d9d99f
  * the same pointer through each time after, and it'll return them in sorted
d9d99f
- * order. */
d9d99f
+ * order as defined in the BLS fragment file */
d9d99f
 static char *bls_get_val(struct bls_entry *entry, const char *keyname, int *last)
d9d99f
 {
d9d99f
-  char *foo = (char *)"";
d9d99f
-  struct keyval *kv = NULL, **kvp, key = {keyname, foo}, *keyp = &ke;;
d9d99f
+  int idx, start = 0;
d9d99f
+  struct keyval *kv = NULL;
d9d99f
 
d9d99f
-  /* if we've already found an entry that matches, just iterate */
d9d99f
-  if (last && *last >= 0)
d9d99f
-    {
d9d99f
-      int next = ++last[0];
d9d99f
+  if (last)
d9d99f
+    start = *last + 1;
d9d99f
 
d9d99f
-      if (next == entry->nkeyvals)
d9d99f
-	{
d9d99f
-done:
d9d99f
-	  *last = -1;
d9d99f
-	  return NULL;
d9d99f
-	}
d9d99f
+  for (idx = start; idx < entry->nkeyvals; idx++) {
d9d99f
+    kv = entry->keyvals[idx];
d9d99f
 
d9d99f
-      kv = entry->keyvals[next];
d9d99f
-      if (grub_strcmp (keyname, kv->key))
d9d99f
-	goto done;
d9d99f
+    if (!grub_strcmp (keyname, kv->key))
d9d99f
+      break;
d9d99f
+  }
d9d99f
 
d9d99f
-      return kv->val;
d9d99f
-    }
d9d99f
+  if (idx == entry->nkeyvals) {
d9d99f
+    if (last)
d9d99f
+      *last = -1;
d9d99f
+    return NULL;
d9d99f
+  }
d9d99f
 
d9d99f
-  kvp = grub_bsearch(&keyp, &entry->keyvals[0], entry->nkeyvals,
d9d99f
-		    sizeof (struct keyval *), keyval_cmp, NULL);
d9d99f
-  if (kvp)
d9d99f
-    kv = *kvp;
d9d99f
+  if (last)
d9d99f
+    *last = idx;
d9d99f
 
d9d99f
-  if (kv)
d9d99f
-    {
d9d99f
-      /* if we've got uninitialized but present state, track back until we find
d9d99f
-       * the first match */
d9d99f
-      if (last)
d9d99f
-	{
d9d99f
-	  grub_dprintf("blscfg", "%s trying to find another entry because last was set\n", __func__);
d9d99f
-	  /* figure out the position of this entry in the array */
d9d99f
-	  int idx;
d9d99f
-	  for (idx = 0 ; idx < entry->nkeyvals; idx++)
d9d99f
-	    if (entry->keyvals[idx] == kv)
d9d99f
-	      break;
d9d99f
-	  *last = idx;
d9d99f
-
d9d99f
-	  while (idx > 0)
d9d99f
-	    {
d9d99f
-	      struct keyval *kvtmp = entry->keyvals[idx-1];
d9d99f
-	      if (idx == 0 || grub_strcmp (keyname, kvtmp->key))
d9d99f
-		{
d9d99f
-		  /* if we're at the start, or if the previous entry doesn't
d9d99f
-		   * match, then we're done */
d9d99f
-		  *last = idx;
d9d99f
-		  break;
d9d99f
-		}
d9d99f
-	      else
d9d99f
-		/* but if it does match, keep going backwards */
d9d99f
-		idx--;
d9d99f
-	    }
d9d99f
-	}
d9d99f
-
d9d99f
-      return kv->val;
d9d99f
-    }
d9d99f
-  return NULL;
d9d99f
+  return kv->val;
d9d99f
 }
d9d99f
 
d9d99f
 #define goto_return(x) ({ ret = (x); goto finish; })
d9d99f
@@ -503,9 +454,6 @@ static int read_entry (
d9d99f
 	break;
d9d99f
     }
d9d99f
 
d9d99f
-  grub_qsort(&entry->keyvals[0], entry->nkeyvals, sizeof (struct keyval *),
d9d99f
-	     keyval_cmp, NULL);
d9d99f
-
d9d99f
 finish:
d9d99f
   grub_free (p);
d9d99f