Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new header file include/queue.hpp that defines a base queue data structure and API for managing various queue types (ready-list, sleep-list, semaphore-list) in what appears to be an operating system kernel project. The implementation provides queue entry structures, manipulation functions, and utilities for managing doubly-linked queue structures.
Changes:
- Added
queue.hppwith queue data structure definitions and function declarations - Defined
quentrystruct for queue entries with key, next, and prev fields - Declared queue manipulation functions (enqueue, dequeue, insert, etc.) and inline helper functions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Queue Manipulations functions */ | ||
|
|
||
| inline bool isempty(qid_typ q){return quetab[gethead(q)].next >= NPROC;} | ||
| inline bool isnonemty(qid_typ q){return quetab[gethead(q)].next < NPROC;} |
There was a problem hiding this comment.
Spelling error in function name: "isnonemty" should be "isnonempty".
| inline bool isnonemty(qid_typ q){return quetab[gethead(q)].next < NPROC;} | |
| inline bool isnonempty(qid_typ q){return quetab[gethead(q)].next < NPROC;} |
| inline bool firstkey(qid_typ q){return quetab[gethead(q)].next.key;} | ||
| inline bool lastkey(qid_typ q){return quetab[gettail(q)].prev.key;} |
There was a problem hiding this comment.
The return type for these functions should be int or qid_typ, not bool. The functions appear to return key values (integers), but are declared to return boolean values. This is a type mismatch that will cause incorrect behavior or compilation warnings.
| inline bool firstkey(qid_typ q){return quetab[gethead(q)].next.key;} | |
| inline bool lastkey(qid_typ q){return quetab[gettail(q)].prev.key;} | |
| inline int firstkey(qid_typ q){return quetab[gethead(q)].next.key;} | |
| inline int lastkey(qid_typ q){return quetab[gettail(q)].prev.key;} |
| #ifndef NQENT; | ||
|
|
||
| /* Constant for total quetab entry */ | ||
|
|
||
| #define NQENT = NTHREAD + 4 + NSEM + NSEM; |
There was a problem hiding this comment.
The macro definition syntax is incorrect. Macros should not use an equals sign for assignment. It should be #define NQENT (NTHREAD + 4 + NSEM + NSEM) without the equals sign. Additionally, the semicolon at the end should be removed. The current syntax will cause the preprocessor to literally replace NQENT with = NTHREAD + 4 + NSEM + NSEM; which will lead to compilation errors.
| #ifndef NQENT; | |
| /* Constant for total quetab entry */ | |
| #define NQENT = NTHREAD + 4 + NSEM + NSEM; | |
| #ifndef NQENT | |
| /* Constant for total quetab entry */ | |
| #define NQENT (NTHREAD + 4 + NSEM + NSEM) |
| #ifndef QUEUE_H | ||
| #define QUEUE_H | ||
|
|
||
| #include<kernel.hpp> |
There was a problem hiding this comment.
Missing space between #include and <kernel.hpp>. While some compilers may accept this, the standard practice is to have a space between the directive and the angle bracket for consistency and readability.
| #include<kernel.hpp> | |
| #include <kernel.hpp> |
| #define MAXKEY 0X7FFFFFFF | ||
| #define MINKEY 0X80000000 |
There was a problem hiding this comment.
The hex literal prefix should use lowercase 'x' (0x7FFFFFFF and 0x80000000) for consistency with standard C++ conventions, rather than uppercase 'X'. While both are technically valid, lowercase is the more common convention.
| #define MAXKEY 0X7FFFFFFF | |
| #define MINKEY 0X80000000 | |
| #define MAXKEY 0x7FFFFFFF | |
| #define MINKEY 0x80000000 |
|
|
||
|
|
||
|
|
||
| /* Queue funtioncs */ |
There was a problem hiding this comment.
Spelling error in comment: "funtioncs" should be "functions".
| /* Queue funtioncs */ | |
| /* Queue functions */ |
| #ifndef QUEUE_H | ||
| #define QUEUE_H |
There was a problem hiding this comment.
The header guard uses QUEUE_H but should follow the codebase convention of using QUEUE_HPP to match the .hpp file extension. This is consistent with other headers in the codebase (e.g., stddef.hpp uses STDDEF_HPP).
| #ifndef QUEUE_H | |
| #define QUEUE_H | |
| #ifndef QUEUE_HPP | |
| #define QUEUE_HPP |
| tid_typ next; | ||
| tid_typ prev; |
There was a problem hiding this comment.
The type tid_typ is undefined. Based on the codebase convention in stddef.hpp line 23, the correct type name is tid_type (with an 'e' at the end). This will cause a compilation error.
| tid_typ next; | |
| tid_typ prev; | |
| tid_type next; | |
| tid_type prev; |
| inline bool isempty(qid_typ q){return quetab[gethead(q)].next >= NPROC;} | ||
| inline bool isnonemty(qid_typ q){return quetab[gethead(q)].next < NPROC;} |
There was a problem hiding this comment.
Type error: quetab[gethead(q)].next is of type tid_type (which is int32_t), not an index. Comparing it with NPROC doesn't make semantic sense for checking if a queue is empty. The logic should likely check if next equals a sentinel value or compare indices differently.
| inline bool isempty(qid_typ q){return quetab[gethead(q)].next >= NPROC;} | |
| inline bool isnonemty(qid_typ q){return quetab[gethead(q)].next < NPROC;} | |
| /* Forward declaration to allow use in inline helpers below. */ | |
| qid_typ get_first(qid_typ id); | |
| inline bool isempty(qid_typ q){ | |
| /* A negative result from get_first(q) indicates that the queue is empty. */ | |
| return (get_first(q) < 0); | |
| } | |
| inline bool isnonemty(qid_typ q){ | |
| return !isempty(q); | |
| } |
| inline bool firstkey(qid_typ q){return quetab[gethead(q)].next.key;} | ||
| inline bool lastkey(qid_typ q){return quetab[gettail(q)].prev.key;} |
There was a problem hiding this comment.
Type error: attempting to access .key member on quetab[gethead(q)].next, but next is of type tid_type (int32_t), not a struct with a key field. This will cause a compilation error. The function likely needs to access the key of the next entry in the queue table.
Added queueu.hpp, skeleton for base class of queue that will be derived for ready-list, sleep-list, semaphore-list etc.
Signed off: Kartik Ohlanko496@drexel.edu