[PATCH 2/6] mm: clarify grub_real_malloc

[Daniel Axtens]

When iterating through the singly linked list of free blocks,
grub_real_malloc uses p and q for the current and previous blocks
respectively. This isn't super clear, so swap to using prev and cur.

This makes another quirk more obvious. The comment at the top of
grub_real_malloc might lead you to believe that the function will
allocate from *first if there is space in that block.

It actually doesn't do that, and it can't do that with the current
data structures. If we used up all of *first, we would need to change
the ->next of the previous block to point to *first->next, but we
can't do that because it's a singly linked list and we don't have
access to *first's previous block.

What grub_real_malloc actually does is set *first to the initial
previous block, and *first->next is the block we try to allocate
from. That allows us to keep all the data structures consistent.

Document that.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 grub-core/kern/mm.c | 76 ++++++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
index c070afc621f8..6efabe92df0e 100644
--- a/grub-core/kern/mm.c
+++ b/grub-core/kern/mm.c
@@ -178,13 +178,20 @@ grub_mm_init_region (void *addr, grub_size_t size)
 }
 
 /* Allocate the number of units N with the alignment ALIGN from the ring
-   buffer starting from *FIRST.  ALIGN must be a power of two. Both N and
-   ALIGN are in units of GRUB_MM_ALIGN.  Return a non-NULL if successful,
-   otherwise return NULL.  */
+ * buffer given in *FIRST.  ALIGN must be a power of two. Both N and
+ * ALIGN are in units of GRUB_MM_ALIGN.  Return a non-NULL if successful,
+ * otherwise return NULL.
+ *
+ * Note: because in certain circumstances we need to adjust the ->next
+ * pointer of the previous block, we iterate over the singly linked
+ * list with the pair (prev, cur). *FIRST is our initial previous, and
+ * *FIRST->next is our initial current pointer. So we will actually
+ * allocate from *FIRST->next first and *FIRST itself last.
+ */
 static void *
 grub_real_malloc (grub_mm_header_t *first, grub_size_t n, grub_size_t align)
 {
-  grub_mm_header_t p, q;
+  grub_mm_header_t cur, prev;
 
   /* When everything is allocated side effect is that *first will have alloc
      magic marked, meaning that there is no room in this region.  */
@@ -192,24 +199,24 @@ grub_real_malloc (grub_mm_header_t *first, grub_size_t n, 
grub_size_t align)
     return 0;
 
   /* Try to search free slot for allocation in this memory region.  */
-  for (q = *first, p = q->next; ; q = p, p = p->next)
+  for (prev = *first, cur = prev->next; ; prev = cur, cur = cur->next)
     {
       grub_off_t extra;
 
-      extra = ((grub_addr_t) (p + 1) >> GRUB_MM_ALIGN_LOG2) & (align - 1);
+      extra = ((grub_addr_t) (cur + 1) >> GRUB_MM_ALIGN_LOG2) & (align - 1);
       if (extra)
        extra = align - extra;
 
-      if (! p)
+      if (! cur)
        grub_fatal ("null in the ring");
 
-      if (p->magic != GRUB_MM_FREE_MAGIC)
-       grub_fatal ("free magic is broken at %p: 0x%x", p, p->magic);
+      if (cur->magic != GRUB_MM_FREE_MAGIC)
+       grub_fatal ("free magic is broken at %p: 0x%x", cur, cur->magic);
 
-      if (p->size >= n + extra)
+      if (cur->size >= n + extra)
        {
-         extra += (p->size - extra - n) & (~(align - 1));
-         if (extra == 0 && p->size == n)
+         extra += (cur->size - extra - n) & (~(align - 1));
+         if (extra == 0 && cur->size == n)
            {
              /* There is no special alignment requirement and memory block
                 is complete match.
@@ -222,9 +229,9 @@ grub_real_malloc (grub_mm_header_t *first, grub_size_t n, 
grub_size_t align)
                 | alloc, size=n |          |
                 +---------------+          v
               */
-             q->next = p->next;
+             prev->next = cur->next;
            }
-         else if (align == 1 || p->size == n + extra)
+         else if (align == 1 || cur->size == n + extra)
            {
              /* There might be alignment requirement, when taking it into
                 account memory block fits in.
@@ -241,23 +248,22 @@ grub_real_malloc (grub_mm_header_t *first, grub_size_t n, 
grub_size_t align)
                 | alloc, size=n |        |
                 +---------------+        v
               */
-
-             p->size -= n;
-             p += p->size;
+             cur->size -= n;
+             cur += cur->size;
            }
          else if (extra == 0)
            {
              grub_mm_header_t r;
              
-             r = p + extra + n;
+             r = cur + extra + n;
              r->magic = GRUB_MM_FREE_MAGIC;
-             r->size = p->size - extra - n;
-             r->next = p->next;
-             q->next = r;
+             r->size = cur->size - extra - n;
+             r->next = cur->next;
+             prev->next = r;
 
-             if (q == p)
+             if (prev == cur)
                {
-                 q = r;
+                 prev = r;
                  r->next = r;
                }
            }
@@ -284,32 +290,32 @@ grub_real_malloc (grub_mm_header_t *first, grub_size_t n, 
grub_size_t align)
               */
              grub_mm_header_t r;
 
-             r = p + extra + n;
+             r = cur + extra + n;
              r->magic = GRUB_MM_FREE_MAGIC;
-             r->size = p->size - extra - n;
-             r->next = p;
+             r->size = cur->size - extra - n;
+             r->next = cur;
 
-             p->size = extra;
-             q->next = r;
-             p += extra;
+             cur->size = extra;
+             prev->next = r;
+             cur += extra;
            }
 
-         p->magic = GRUB_MM_ALLOC_MAGIC;
-         p->size = n;
+         cur->magic = GRUB_MM_ALLOC_MAGIC;
+         cur->size = n;
 
          /* Mark find as a start marker for next allocation to fasten it.
             This will have side effect of fragmenting memory as small
             pieces before this will be un-used.  */
          /* So do it only for chunks under 64K.  */
          if (n < (0x8000 >> GRUB_MM_ALIGN_LOG2)
-             || *first == p)
-           *first = q;
+             || *first == cur)
+           *first = prev;
 
-         return p + 1;
+         return cur + 1;
        }
 
       /* Search was completed without result.  */
-      if (p == *first)
+      if (cur == *first)
        break;
     }
 
-- 
2.32.0