From 598f97cd788281b23ed696daecfa174c1d0ad30c Mon Sep 17 00:00:00 2001 From: Niklas Gollenstede Date: Mon, 5 Jan 2026 10:41:27 +0100 Subject: [PATCH] fix make for a21, PIDs are always ints, linter things --- .clang-tidy | 8 +++-- kernel/arch/idt.cc | 14 ++++----- kernel/debug/assert.cc | 2 +- kernel/debug/assert.h | 2 +- kernel/object/queue.h | 47 +++++++++++++++-------------- kernel/sync/message.h | 39 ++++-------------------- kernel/sync/semaphore.h | 8 +++-- kernel/syscall/skeleton.cc | 2 -- kernel/syscall/skeleton.h | 2 +- kernel/thread/thread.cc | 3 +- kernel/thread/thread.h | 12 ++++++-- kernel/user/app1/appl.cc | 2 +- libsys/assert.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++ user/Makefile | 10 ++++--- user/Makefile.app | 2 +- user/imgbuilder.cc | 32 ++++++++++---------- 16 files changed, 155 insertions(+), 104 deletions(-) create mode 100644 libsys/assert.h diff --git a/.clang-tidy b/.clang-tidy index cabaf8c..f90f0de 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,6 +1,6 @@ FormatStyle: google HeaderFilterRegex: '.*' -WarningsAsErrors: 'readability*' +#WarningsAsErrors: 'readability*' Checks: > google-readability-casting, google-explicit-constructor, @@ -9,6 +9,7 @@ Checks: > -readability-else-after-return, -readability-function-cognitive-complexity -readability-identifier-length, + -readability-implicit-bool-conversion, -readability-magic-numbers, bugprone*, -bugprone-easily-swappable-parameters, @@ -19,8 +20,11 @@ Checks: > -cppcoreguidelines-avoid-do-while, -cppcoreguidelines-avoid-magic-numbers, -cppcoreguidelines-avoid-non-const-global-variables, + -cppcoreguidelines-non-private-member-variables-in-classes, -cppcoreguidelines-owning-memory, -cppcoreguidelines-pro-*, CheckOptions: readability-simplify-boolean-expr.IgnoreMacros: 'true' - readability-identifier-length.IgnoreNames: 'g,me' + readability-identifier-length.IgnoredVariableNames: '^(g|id|me)$' + readability-identifier-length.IgnoredParameterNames: '^(id|ms|p[1-5])$' + readability-identifier-length.IgnoredLoopCounterNames: '.*' diff --git a/kernel/arch/idt.cc b/kernel/arch/idt.cc index 22c21ba..f30394c 100644 --- a/kernel/arch/idt.cc +++ b/kernel/arch/idt.cc @@ -10,13 +10,9 @@ constinit struct InterruptDescriptor idt[256] = {}; // Struct used for loading (the address of) the Interrupt Descriptor Table into // the IDT-Register struct Register { - uint16_t limit; // Address of the last valid byte (relative to base) - struct InterruptDescriptor* base; - explicit Register(uint8_t max = 255) { - limit = - (max + static_cast(1)) * sizeof(InterruptDescriptor) - 1; - base = idt; - } + uint16_t limit = + sizeof(idt) - 1; // Address of the last valid byte (relative to base) + struct InterruptDescriptor* base = idt; } __attribute__((packed)); static_assert(sizeof(InterruptDescriptor) == 16, @@ -26,11 +22,11 @@ static_assert(alignof(decltype(idt)) % 8 == 0, "IDT must be 8 byte aligned!"); void load() { // Create structure required for writing to idtr and load via lidt - Register idtr(Core::Interrupt::VECTORS - 1); + Register idtr; asm volatile("lidt %0\n\t" ::"m"(idtr)); } void set(Core::Interrupt::Vector vector, InterruptDescriptor descriptor) { - idt[(uint8_t)vector] = descriptor; + idt[static_cast(vector)] = descriptor; } } // namespace IDT diff --git a/kernel/debug/assert.cc b/kernel/debug/assert.cc index 678e3c5..066d882 100644 --- a/kernel/debug/assert.cc +++ b/kernel/debug/assert.cc @@ -1,7 +1,7 @@ #include "assert.h" #include "../arch/core.h" -#include "output.h" +#include "./output.h" [[noreturn]] void assertion_failed(const char* exp, const char* func, const char* file, int line) { diff --git a/kernel/debug/assert.h b/kernel/debug/assert.h index d6546b7..e28de40 100644 --- a/kernel/debug/assert.h +++ b/kernel/debug/assert.h @@ -15,7 +15,7 @@ */ #pragma once -#include "../types.h" +#include "types.h" #ifndef STRINGIFY /*! \def STRINGIFY(S) diff --git a/kernel/object/queue.h b/kernel/object/queue.h index 7f49815..1cba188 100644 --- a/kernel/object/queue.h +++ b/kernel/object/queue.h @@ -25,18 +25,22 @@ class Queue { /// Function pointer to the get_link function, called whenever the /// next pointer array is referenced const NextFunc get_link = default_get_link; - /// head points to the first element (the one returned on first dequeue). - /// Can be nullptr if the queue is empty. + /// `head` points to the first element (the one returned on first dequeue). + /// Can be `nullptr` if the queue is empty. T* head = nullptr; - /// tail points to the last element (the one last added). - /// Is only valid if head != nullptr + /// `tail` points to the last element (the one last added). + /// Is only valid if `head != nullptr`. T* tail = nullptr; // Prevent copies and assignments Queue(const Queue&) = delete; + Queue(Queue&&) = delete; Queue& operator=(const Queue&) = delete; + Queue& operator=(Queue&&) = delete; public: + ~Queue() = default; + /*! \brief Minimal forward iterator * You can use this iterator to iterate the queue like a normal STL * container. It only supports forward iteration, since the queue is single @@ -44,11 +48,11 @@ class Queue { */ class Iterator { private: - Queue& queue; + Queue* queue; T* current; friend class Queue; Iterator(Queue& queue, T* current) - : queue(queue), current(current) {} + : queue(&queue), current(current) {} public: Iterator operator+(unsigned num) { @@ -57,21 +61,21 @@ class Queue { } T* temp = current; while (num--) { - temp = queue.next(*temp); + temp = queue->next(*temp); } - return Iterator(queue, temp); + return Iterator(*queue, temp); } // pre increment Iterator& operator++() { - current = queue.next(*current); + current = queue->next(*current); return *this; } // post increment Iterator operator++(int) { auto temp = Iterator(queue, current); - current = queue.next(*current); + current = queue->next(*current); return temp; } @@ -84,8 +88,6 @@ class Queue { bool operator!=(const Iterator& other) { return !(*this == other); } }; - constexpr Queue(Queue&&) = default; - /*! \brief Constructor * \param[in] get_link A function pointer to the get_link, i.e. a function * which returns a pointer to the next-pointer of an @@ -97,11 +99,10 @@ class Queue { "get_link function pointer must not be nullptr!"); } - /*! \brief Enqueues the provided item at the end of the queue. If the - *element is already contained in the queue, false will be returned - * \param[in] item element to be appended (enqueued). - * \return false if the element already was enqueued (and nothing was done) - * or not (and it is now enqueued, then true) + /*! \brief Enqueues the provided item at the end of the queue. + * \param[in] item Element to be appended (enqueued). + * \return `false` if the element already was already in this (or a + * different) queue (and nothing was done), `true` otherwise. */ bool enqueue(T& item) { T** nextptr = get_link(item); @@ -214,7 +215,7 @@ class Queue { T* remove(T* that) { if (!that) return nullptr; T* cur = head; - T** next_link; + T** next_link = nullptr; if (head == that) { head = *get_link(*head); @@ -258,14 +259,14 @@ class Queue { * the pointer. */ template -OutputStream& operator<<(OutputStream& os, Queue& queue) { - os << "{"; +OutputStream& operator<<(OutputStream& out, Queue& queue) { + out << "{"; for (typename Queue::Iterator it = queue.begin(); it != queue.end(); ++it) { - os << *it; + out << *it; if (it + 1 != queue.end()) { - os << ", "; + out << ", "; } } - return os << "}"; + return out << "}"; } diff --git a/kernel/sync/message.h b/kernel/sync/message.h index 3f019ff..5f09735 100644 --- a/kernel/sync/message.h +++ b/kernel/sync/message.h @@ -3,42 +3,13 @@ #include "../types.h" struct Message { - const size_t pid; ///< Sender Thread ID of message + int sender; ///< Sender Thread ID of message - const uintptr_t sbuffer; ///< Send buffer - const size_t ssize; ///< Send buffer size + uintptr_t sbuffer; ///< Send buffer + size_t ssize; ///< Send buffer size - const uintptr_t rbuffer; ///< Receive buffer - const size_t rsize; ///< Receive buffer size + uintptr_t rbuffer; ///< Receive buffer + size_t rsize; ///< Receive buffer size Message *queue_link = nullptr; ///< Next message in the message queue - - /*! \brief Constructor - * \param pid Sender Thread ID of message - * \param sbuffer Send buffer - * \param ssize Send buffer size - * \param rbuffer Receive buffer - * \param rsize Receive buffer size - */ - explicit Message(int pid, uintptr_t sbuffer = 0, size_t ssize = 0, - uintptr_t rbuffer = 0, size_t rsize = 0) - : pid(pid), - sbuffer(sbuffer), - ssize(ssize), - rbuffer(rbuffer), - rsize(rsize) {} - - /*! \brief Helper to retrieve (and remove) a message from the queue - * \param pid Thread id of message - * \param queue Queue with message - * \return Pointer to message or nullptr - */ - static Message *dequeueByPID(size_t pid, Queue &queue) { - for (Message *m : queue) { - if (m->pid == pid) { - return queue.remove(m); - } - } - return nullptr; - } }; diff --git a/kernel/sync/semaphore.h b/kernel/sync/semaphore.h index cb0a9bd..6200229 100644 --- a/kernel/sync/semaphore.h +++ b/kernel/sync/semaphore.h @@ -24,15 +24,17 @@ class Thread; class Semaphore { // Prevent copies and assignments Semaphore(const Semaphore&) = delete; + Semaphore(Semaphore&&) = delete; Semaphore& operator=(const Semaphore&) = delete; + Semaphore& operator=(Semaphore&&) = delete; unsigned counter; Queue waiting; public: - /*! \brief Constructor; initialized the counter with provided value `c` - * \param c Initial counter value + /*! \param init Initial counter value */ - explicit Semaphore(unsigned c = 0) : counter(c) {} + explicit Semaphore(unsigned init = 0) : counter(init) {} + ~Semaphore() = default; /*! \brief Wait for access to the critical area. * diff --git a/kernel/syscall/skeleton.cc b/kernel/syscall/skeleton.cc index 33e0b5c..e790e9b 100644 --- a/kernel/syscall/skeleton.cc +++ b/kernel/syscall/skeleton.cc @@ -1,8 +1,6 @@ #include "syscall/skeleton.h" -#include "debug/kernelpanic.h" #include "debug/output.h" -#include "device/textstream.h" #include "interrupt/guard.h" #include "sync/semaphore.h" #include "thread/scheduler.h" diff --git a/kernel/syscall/skeleton.h b/kernel/syscall/skeleton.h index 5f9f831..7a33ec8 100644 --- a/kernel/syscall/skeleton.h +++ b/kernel/syscall/skeleton.h @@ -22,7 +22,7 @@ void sem_destroy(Vault &vault, size_t id); void sem_signal(Vault &vault, size_t id); void sem_wait(Vault &vault, size_t id); void exit(Vault &vault); -void kill(Vault &vault, size_t pid); +void kill(Vault &vault, int pid); } // namespace Skeleton } // namespace Syscall diff --git a/kernel/thread/thread.cc b/kernel/thread/thread.cc index b53c678..c6af7d5 100644 --- a/kernel/thread/thread.cc +++ b/kernel/thread/thread.cc @@ -6,8 +6,7 @@ #include "../interrupt/guard.h" #include "debug/output.h" -// counter for ID -static size_t idCounter = 1; +static int idCounter = 1; // counter for task IDs void Thread::kickoff(uintptr_t param1, uintptr_t param2, uintptr_t param3) { Thread *thread = reinterpret_cast(param1); diff --git a/kernel/thread/thread.h b/kernel/thread/thread.h index 8149670..35f04e2 100644 --- a/kernel/thread/thread.h +++ b/kernel/thread/thread.h @@ -28,14 +28,12 @@ class Thread { /*! \brief pointer to the next element of the readylist */ Thread* queue_link; - friend class Queue; friend class Semaphore; /*! \brief Memory reserved for this threads stack */ alignas(16) char reserved_stack_space[STACK_SIZE]; - protected: /*! \brief Context of the thread, used for saving and restoring the register * values when context switching. */ @@ -56,17 +54,25 @@ class Thread { * \param param1 Thread to be started * \param param2 Second parameter (will be used later) * \param param3 Third parameter (will be used later) + * */ static void kickoff(uintptr_t param1, uintptr_t param2, uintptr_t param3); public: /*! \brief Unique thread id */ - const size_t id; + const int id; /*! \brief Marker for a dying thread */ volatile bool kill_flag; + // Naively moving or copying esp. the (user) stack of a thread would be a + // bad idea: + Thread(const Thread&) = delete; + Thread(Thread&&) = delete; + Thread& operator=(const Thread&) = delete; + Thread& operator=(Thread&&) = delete; + /*! \brief Constructor * Initializes the context using \ref prepareContext with the thread's * stack space. diff --git a/kernel/user/app1/appl.cc b/kernel/user/app1/appl.cc index 99791d8..869a768 100644 --- a/kernel/user/app1/appl.cc +++ b/kernel/user/app1/appl.cc @@ -23,7 +23,7 @@ void Application::action() { // NOLINT // Make sure that we can use kout exclusively due to the hardware cursor // otherwise we'd get a word jumble koutsem.p(Guard::enter().vault()); - kout.setPos(0U, id); + kout.setPos(0U, static_cast(id)); kout << i; kout.flush(); koutsem.v(Guard::enter().vault()); diff --git a/libsys/assert.h b/libsys/assert.h new file mode 100644 index 0000000..e28de40 --- /dev/null +++ b/libsys/assert.h @@ -0,0 +1,74 @@ +// vim: set noet ts=4 sw=4: + +/*! \file + * \brief Contains several macros usable for making assertions + * + * Depending on the type of assertion (either static or at runtime), a failing + * assertion will trigger an error. For static assertion, this error will be + * shown at compile time and abort compilation. Runtime assertions will trigger + * a message containing details about the error occurred and will make the CPU + * die. + */ + +/*! + * \defgroup debug Debugging functions + */ + +#pragma once +#include "types.h" + +#ifndef STRINGIFY +/*! \def STRINGIFY(S) + * \brief Converts a macro parameter into a string + * \ingroup debug + * \param S Expression to be converted + * \return stringified version of S + */ +#define STRINGIFY(S) #S +#endif + +/*! \def assert_size(TYPE, SIZE) + * \brief Statically ensure (at compile time) that a data type (or variable) + * has the expected size. + * + * \ingroup debug + * \param TYPE The type to be checked + * \param SIZE Expected size in bytes + */ +#define assert_size(TYPE, SIZE) \ + static_assert(sizeof(TYPE) == (SIZE), "Wrong size for " STRINGIFY(TYPE)) + +/*! \def assert(EXP) + * \brief Ensure (at execution time) an expression evaluates to `true`, print + * an error message and stop the CPU otherwise. + * + * \ingroup debug + * \param EXP The expression to be checked + */ +#ifdef NDEBUG +#define assert(EXP) ((void)0) +#else +#define assert(EXP) \ + do { \ + if (__builtin_expect(!(EXP), 0)) { \ + assertion_failed(STRINGIFY(EXP), __func__, __FILE__, __LINE__); \ + } \ + } while (false) + +/*! \brief Handles a failed assertion + * + * This function will print a message containing further information about the + * failed assertion and stops the current CPU permanently. + * + * \note This function should never be called directly, but only via the macro + * `assert`. + * + * + * \param exp Expression that did not hold + * \param func Name of the function in which the assertion failed + * \param file Name of the file in which the assertion failed + * \param line Line in which the assertion failed + */ +[[noreturn]] void assertion_failed(const char* exp, const char* func, + const char* file, int line); +#endif diff --git a/user/Makefile b/user/Makefile index bd8767f..7d3d491 100644 --- a/user/Makefile +++ b/user/Makefile @@ -19,14 +19,16 @@ $(APPS): $(BUILDDIR)/init.o # recipe for compiling imgbuilder $(IMGBUILDER): imgbuilder.cc $(MAKEFILE_LIST) - @echo "CC $@" - @if test \( ! \( -d $(@D) \) \) ;then mkdir -p $(@D);fi + @echo "CXX $@" + @mkdir -p $(@D) $(VERBOSE) $(CXX) -std=c++23 -o $@ $< -# recipe for building the final oostubs image +.ONESHELL: +# recipe for building the initrd image $(INITRD): $(APPS) $(IMGBUILDER) @echo "IMGBUILD $@" - @if test \( ! \( -d $(@D) \) \) ;then mkdir -p $(@D);fi + @mkdir -p $(@D) + if [ -z "$(APPS)" ] ; then touch $@ ; exit 0 ; fi $(VERBOSE) $(IMGBUILDER) $(addsuffix $(BUILDDIR)/app.img, $(APPS)) > $@ lint:: diff --git a/user/Makefile.app b/user/Makefile.app index a88f5b1..8351c83 100644 --- a/user/Makefile.app +++ b/user/Makefile.app @@ -41,7 +41,7 @@ $(BUILDDIR)/app: $(INITOBJ) $(ASM_OBJECTS) $(CC_OBJECTS) $(LINKER_SCRIPT) $(MAK $(BUILDDIR)/app.img: $(BUILDDIR)/app @echo "OBJCOPY $@" @if ! nm $< | grep "4000000 T start" >/dev/null 2>&1 ; then echo "Symbol 'start' is not first address" ; exit 1 ; fi - @if test \( ! \( -d $(@D) \) \) ;then mkdir -p $(@D);fi + @mkdir -p $(@D) $(VERBOSE) objcopy -O binary --set-section-flags .bss=alloc,load,contents $< $@ # -------------------------------------------------------------------------- diff --git a/user/imgbuilder.cc b/user/imgbuilder.cc index 19d16a4..535cc73 100644 --- a/user/imgbuilder.cc +++ b/user/imgbuilder.cc @@ -10,7 +10,7 @@ #include #include -constexpr size_t block_size = 4096; +constexpr std::streamsize block_size = 4096; const uint8_t zeros[block_size] = {}; template @@ -20,7 +20,7 @@ static void die(std::format_string fmt, Args &&...args) { exit(EXIT_FAILURE); } -static void writeBuffer(const void *data, size_t size) { +static void writeBuffer(const void *data, std::streamsize size) { std::cout.write(reinterpret_cast(data), size); } @@ -33,35 +33,33 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - // HEADER: Number of apps + size of each app - std::array size{0}; - size[0] = static_cast(argc - 1); - + // HEADER: Number of apps + size of each app in bytes + std::array sizes{0}; + sizes[0] = static_cast(argc - 1); for (size_t i = 1; i < argc; ++i) { - struct stat sb; - if (stat(argv[i], &sb) != 0) die("stat"); - // 4 GB Limit - if (sb.st_size >= UINT32_MAX) { + struct stat file; + if (stat(argv[i], &file) != 0) die("stat"); + if (file.st_size >= UINT32_MAX) { // 4 GB Limit errno = EFBIG; die("stat"); } - size[i] = sb.st_size; + sizes[i] = file.st_size; } - writeBuffer(size.data(), block_size); + writeBuffer(sizes.data(), block_size); // DATA: Each App for (size_t i = 1; i < argc; ++i) { - std::vector buf(size[i]); + std::vector buf(sizes[i]); std::ifstream input(argv[i], std::ios::binary); if (!input) die("fopen"); - input.read(buf.data(), size[i]); + input.read(buf.data(), sizes[i]); - writeBuffer(buf.data(), size[i]); + writeBuffer(buf.data(), sizes[i]); // Fill to block size, if required - if (size[i] % block_size != 0) - writeBuffer(zeros, block_size - (size[i] % block_size)); + if (sizes[i] % block_size != 0) + writeBuffer(zeros, block_size - (sizes[i] % block_size)); } std::cout.flush();