Thread: I would like a logic check, please.

I've got this for my pmm, but it's ugly. Speed is very important, so I tried to make it as simple as possible, but also very fast. I'm kinda dissatisfied with it and not sure it will function as it's supposed to. I'm nowhere near(weeks away) from being able to compile, so I need some reassurance that it's correct before I proceed with development. If anyone can help me simplify it, or point out a bug, I'd greatly appreciate it.

Code:

unsigned int get_pages(int pages)
{
    int i,j = 0;
    for(i = reserved_low_pages + 1;i + pages <= total_pages;i += (j + 1))
        for(j = 0;j <= pages;j++)
            if(j == pages)
            {
                alloc_pages(i, pages);
                return i;
            }
            else if(check_page(i + j))
                break;
    panic("Out of memory!");
    return 0;
}

Pages are managed starting at 1(ie first page is identified with 1, not 0).

That was my original design, but the ambiguity of whether or not it would execute the if w/o me wanting it to caused me to redesign it to explicitly with the if. How do I specify I want it to do nothing? When there are no braces, doesn't it assume the next thing it finds is the instruction to loop?

Would putting a ";" after the for expression be clearer? It doesn't seem to cause a warning or error in a test I made. Is this a GCC extension?

Code:

int main()
{
    int i;
    for(i = 0;i < 5;i++);
    if(i == 4)
        break;
}

The break is there to test if it is inside the for, it isn't because I get an error saying the break is outside of any loop.

Other than the ambiguous for, can you see any bug in my algo?

And btw, thanks for your help on the for thing.

Alloc and check are very trivial. Alloc checks if a page is used, if not it marks it used, if it is, it panics. Check just checks if a bit in a bitmap is set and returns 1 or 0 indicating one way or another.

I've renamed stuff. I didn't like that the names resembled early Linux as much as they did.
pmem.c:

Code:

#include <kernel/mem/pmem.h>
#include <kernel/panic.h>

unsigned int high_mem, low_mem, reserved_low_pages;
unsigned int pbmp[MAX_PAGES/32];

#define pbmpset(page) pbmp[(unsigned int)page / 32] |= (1 << page % 32)
#define pbmpclear(page) pbmp[(unsigned int)page / 32] &= ~(1 << page % 32)
#define pbmpcheck(page) pbmp[(unsigned int)page / 32] & ~(1 << page % 32)

int palloc(unsigned int page)
{
	if(pcheck(page) != MEM_RESERVED)
	{
		pbmpset(page);
		return PALLOC_SUCCESS;
	}
	else
	{
		panic("Allocating used page!");
		return PALLOC_FAIL;
	}
}

int palloc_m(unsigned int start, int len)
{
	unsigned int i;
	if(pcheck_m(start, len) == MEM_RESERVED)
		return PALLOC_FAIL;
	for(i = start;i < start + len;i++)
		palloc(i);
	return PALLOC_SUCCESS;
}

int pfree(unsigned int page)
{
	if(pcheck(page) == MEM_RESERVED)
		pbmpclear(page);
	else
	{
		panic("Freeing free page!");
		return PFREE_FAIL;
	}
	return PFREE_SUCCESS;
}

int pfree_m(unsigned int start, int len)
{
	int i;
	for(i = start;i < start + len;i++)
		if(pfree(i))
			return PFREE_FAIL;
	return PFREE_SUCCESS;
}


int pcheck(unsigned int page)
{
	if(pbmpcheck(page))
		return MEM_RESERVED;
	else
		return MEM_FREE;
}

int pcheck_m(unsigned int start, int len)
{
	unsigned int i;
	for(i = start;i < start + len;i++)
		if(pcheck(i) == MEM_RESERVED)
			return MEM_RESERVED;
	return MEM_FREE;
}

unsigned int pget()
{
	unsigned int i;
	for(i = reserved_low_pages + 1;i <= high_page;i++)
		if(pcheck(i) == MEM_FREE)
		{
			palloc(i);
			return i;
		}
	panic("Out of memory!");
	return PGET_FAIL;
}

unsigned int pget_m(int pages)
{
	unsigned int i, j = 0;
	for(i = reserved_low_pages + 1;i + pages <= high_page;i += (j + 1))
		for(j = 0;j <= pages;j++);
		if(j == pages)
		{
			if(palloc_m(i, pages) == 0)
				return i;
			else
				return PGET_FAIL;
		}
	panic("Out of memory!");
	return 0;
}

I would if it was for external use, but since typing caps is twice the work, I'm gonna take the easy way out.