Having trouble understanding free() from K&R

Question

I'm having a lot of trouble understanding what's going on inside the free function shown on chapter 8.7 from K&R, here's the full code and some information on how the program operates:

1. The blocks are kept in order of increasing storage address, and the last block (highest address) points to the first.
2. When a request is made, the free list is scanned until a big-enough block is found.
3. Freeing also causes a search of the free list, to find the proper place to insert the block being freed. If the block being freed is adjacent to a free block on either side, it is coalesced with it into a single bigger block, so storage does not become too fragmented. Determining adjacency is easy because the free list is maintained in order of increasing address.
4. A free block contains:
   • A pointer to the next block in the chain
   • A record of the size of the block
   • The free space itself
5. The control information at the beginning is called the "header". To simplify alignment, all blocks are multiples of the header size, and the header is aligned properly.

#include <unistd.h>

#define NULL ((void *)0)

typedef long Align; /* for alignment to long boundary */

union header {      /* block header */
    struct {
        union header *ptr;  /* next block if on free list */
        unsigned size;      /* size of this block */
    } s;
    Align x;        /* force alignment of blocks */
};
typedef union header Header;

static Header base;             /* empty list to get started */
static Header *freep = NULL;    /* start of free list */

/* malloc: general-purpose storage allocator */
void *kr_malloc(unsigned nbytes)
{
    Header *p, *prevp;
    Header *morecore(unsigned);
    unsigned nunits;
    nunits = (nbytes+sizeof(Header)-1)/sizeof(Header) + 1;

    if ((prevp = freep) == NULL) { /* no free list yet */
        base.s.ptr = freep = prevp = &base;
        base.s.size = 0;
    }

    for (p = prevp->s.ptr; ; prevp = p, p = p->s.ptr) {
        if (p->s.size >= nunits) {      /* big enough */
            if (p->s.size == nunits)    /* exactly */
                prevp->s.ptr = p->s.ptr;
            else {
                p->s.size -= nunits;
                p += p->s.size;
                p->s.size = nunits;
            }
            freep = prevp;
            return p+1;
        }
        if (p == freep) /* wrapped around free list */
            if ((p = morecore(nunits)) == NULL)
                return NULL; /* none left */
    }
}

#define NALLOC 1024 /* minimum #units to request */

/* morecore: ask system for more memory */
Header *morecore(unsigned nu)
{
    char *cp;
    void kr_free(void *);
    Header *up;

    if (nu < NALLOC)
        nu = NALLOC;

    cp = sbrk(nu * sizeof(Header));
    if (cp == (char *) -1) /* no space at all */ 
        return NULL;

    up = (Header *) cp;
    up->s.size = nu;

    kr_free(up+1);
    return freep;
}

/* free: put block ap in free list */
void kr_free(void *ap)
{
    Header *bp, *p;
    bp = (Header *)ap - 1;  /* point to block header */

    for (p = freep; !(bp > p && bp < p->s.ptr); p = p->s.ptr)
        if (p >= p->s.ptr && (bp > p || bp < p->s.ptr))
            break; /* freed block at start or end of arena */

    if (bp + bp->s.size == p->s.ptr) { /* join to upper nbr */
        bp->s.size += p->s.ptr->s.size;
        bp->s.ptr = p->s.ptr->s.ptr;
    } else
        bp->s.ptr = p->s.ptr;
    if (p + p->s.size == bp) { /* join to lower nbr */
        p->s.size += bp->s.size;
        p->s.ptr = bp->s.ptr;
    } else
        p->s.ptr = bp;
    freep = p;
}

/* free: put block ap in free list */
void kr_free(void *ap)
{
    Header *bp, *p;
    bp = (Header *)ap - 1;  /* point to block header */

So.. bp is gonna point to the header of the block passed in as an argument (ap) and p is gonna be the variable used to iterate over the linked list, so far so good.

The first loop inside free is easy, just keep iterating over the linked list until bp is between p and p->s.ptr (p / bp / p->s.ptr) so you can insert bp where it belongs.

for (p = freep; !(bp > p && bp < p->s.ptr); p = p->s.ptr)

And here's where I'm starting to have trouble:

if (p >= p->s.ptr && (bp > p || bp < p->s.ptr))

I understand that p > p->s.ptr is a condition that will only be true if p (the pointer that is being used to iterate over the linked list) is the last free block in the linked list, and p == p->s.ptr will only happened if p is the only block of free memory available in the linked list, so when you say:

p >= p->s.ptr

you are checking if p is the last or only free block in the linked list why would you want to check for this case? And also what's the point of:

... && (bp > p || bp < p->s.ptr))? What is it that you are checking here? And why do you break out of the loop if the condition is true?

    if (bp + bp->s.size == p->s.ptr) { /* join to upper nbr */
        bp->s.size += p->s.ptr->s.size;
        bp->s.ptr = p->s.ptr->s.ptr;
    } else
        bp->s.ptr = p->s.ptr;

No problem here, you are just coalescing the block passed in as an argument (ap) with p->s.ptr if ap and p->s.ptr are next to each other (ap / p->s.ptr).

    if (p + p->s.size == bp) { /* join to lower nbr */
        p->s.size += bp->s.size;
        p->s.ptr = bp->s.ptr;
    } else
        p->s.ptr = bp;

You are doing the same as above but this time coalescing ap with p.

Answer 1

Testing for (bp > p || bp < p->s.ptr) when p points to the last block in the free list is necessary to only break if the block to be freed is after the last block or before the first block of the free list.

This loop starts at freep, which is not necessarily the free block with the lowest address as malloc sets freep = prevp; when it returns a block carved from the free list, and free sets freep = p; the address of the last freed block.

Setting freep this way improves efficiency when blocks are freed in increasing order of addresses, which is the case when they are freed in the same order as allocation in a clean arena assuming no intervening calls to free.