Skip to content

fix(k-alloc.cc/kalloc): remove comment about page alignment of page size#19

Open
pratikpc wants to merge 1 commit into
CS161:mainfrom
pratikpc:patch-1
Open

fix(k-alloc.cc/kalloc): remove comment about page alignment of page size#19
pratikpc wants to merge 1 commit into
CS161:mainfrom
pratikpc:patch-1

Conversation

@pratikpc
Copy link
Copy Markdown

@pratikpc pratikpc commented Nov 9, 2024

I was just looking at the course and the resources shared online.

It immensely helped me to:-

  1. Reinforce my understanding of items I had studied back in the day in college or learnt during work
  2. Learnt about new items that I either hadn't studied about or forgotten :-P

I quite liked the way everything is presented, with a focus on simplicity and readability which means the lectures can with some googling, be followed without having to resort to videos or notes.

However, while I was looking at the problem statement, I noticed but a minor issue with the comments in k-alloc.cc

//    If `sz` is a multiple of `PAGESIZE`, the returned pointer is guaranteed
//    to be page-aligned.

The comment could cause confusion because:-

  1. kalloc's first line checks whether size is within (0, PAGESIZE)
 if (sz == 0 || sz > PAGESIZE) {
        return nullptr;
    }
  1. So it sort of applies only for sz=PAGESIZE

NOTE:-

  1. By all accounts, the comment is technically correct because nullptr is 0. Just confusing.
  2. I believe the check was added because the code originally didn't have it, and it would have caused issues upon incrementing next_free_pa
            next_free_pa += PAGESIZE;
  1. It could possibly be resolved and the check removed with a roundup logic, but while it would improve correctness, I guess the same hasn't been done to keep the code simple and more readable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant