From bb0d30610e13b4ee9e0a7b1ff5983c2351247f37 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Sun, 13 Mar 2022 17:59:56 -0700 Subject: [PATCH] [util] Add util::format replacement for snprintf The printf library I have been using, while useful, has way more than I need in it, and had comparably huge stack space requirements. This change adds a new util::format() which is a replacement for snprintf, but with only the features used by kernel logging. The logger has been changed to use it, as well as the few instances of snprintf in the interrupt handling code before calling kassert. Also part of this change: the logger's (now vestigial) immediate output handling code is removed, as well as the "sequence" field on log message headers. --- src/kernel/interrupts.cpp | 11 ++- src/kernel/kernel.module | 1 - src/kernel/logger.cpp | 31 +++---- src/kernel/logger.h | 19 +---- src/libraries/util/format.cpp | 133 +++++++++++++++++++++++++++++++ src/libraries/util/util.module | 2 + src/libraries/util/util/format.h | 16 ++++ src/user/srv.logger/main.cpp | 1 - 8 files changed, 167 insertions(+), 47 deletions(-) create mode 100644 src/libraries/util/format.cpp create mode 100644 src/libraries/util/util/format.h diff --git a/src/kernel/interrupts.cpp b/src/kernel/interrupts.cpp index fe1750c..f05cd56 100644 --- a/src/kernel/interrupts.cpp +++ b/src/kernel/interrupts.cpp @@ -1,5 +1,5 @@ #include - +#include #include "assert.h" #include "cpu.h" @@ -10,7 +10,6 @@ #include "logger.h" #include "memory.h" #include "objects/process.h" -#include "printf/printf.h" #include "scheduler.h" #include "vm_space.h" @@ -105,10 +104,10 @@ isr_handler(cpu_state *regs) (!ti) ? "GDT" : "LDT"; - snprintf(message, sizeof(message), "General Protection Fault, error:0x%lx%s %s[%d]", + util::format({message, sizeof(message)}, "General Protection Fault, error:0x%lx%s %s[%d]", regs->errorcode, regs->errorcode & 1 ? " external" : "", table, index); } else { - snprintf(message, sizeof(message), "General Protection Fault, error:%lx%s", + util::format({message, sizeof(message)}, "General Protection Fault, error:%lx%s", regs->errorcode, regs->errorcode & 1 ? " external" : ""); } kassert(false, message, regs); @@ -129,7 +128,7 @@ isr_handler(cpu_state *regs) if (cr2 && space.handle_fault(cr2, ft)) break; - snprintf(message, sizeof(message), + util::format({message, sizeof(message)}, "Page fault: %016lx%s%s%s%s%s", cr2, (regs->errorcode & 0x01) ? " present" : "", (regs->errorcode & 0x02) ? " write" : "", @@ -157,7 +156,7 @@ isr_handler(cpu_state *regs) return; default: - snprintf(message, sizeof(message), "Unknown interrupt 0x%lx", regs->interrupt); + util::format({message, sizeof(message)}, "Unknown interrupt 0x%lx", regs->interrupt); kassert(false, message, regs); } diff --git a/src/kernel/kernel.module b/src/kernel/kernel.module index e2543b4..4429e4a 100644 --- a/src/kernel/kernel.module +++ b/src/kernel/kernel.module @@ -42,7 +42,6 @@ kernel = module("kernel", "page_table.cpp", "page_tree.cpp", "pci.cpp", - "printf/printf.c", "scheduler.cpp", "smp.cpp", "smp.s", diff --git a/src/kernel/logger.cpp b/src/kernel/logger.cpp index 920bfc3..26500f8 100644 --- a/src/kernel/logger.cpp +++ b/src/kernel/logger.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include "assert.h" @@ -8,7 +9,6 @@ #include "memory.h" #include "objects/system.h" #include "objects/thread.h" -#include "printf/printf.h" static uint8_t log_buffer[128 * 1024]; @@ -18,9 +18,6 @@ static uint8_t log_buffer[128 * 1024]; static util::no_construct __g_logger_storage; log::logger &g_logger = __g_logger_storage.value; -// For printf.c -extern "C" void putchar_(char c) {} - namespace log { @@ -33,19 +30,15 @@ const char *logger::s_area_names[] = { nullptr }; -logger::logger(logger::immediate_cb output) : - m_buffer(nullptr, 0), - m_immediate(output), - m_sequence(0) +logger::logger() : + m_buffer(nullptr, 0) { memset(&m_levels, 0, sizeof(m_levels)); s_log = this; } -logger::logger(uint8_t *buffer, size_t size, logger::immediate_cb output) : - m_buffer(buffer, size), - m_immediate(output), - m_sequence(0) +logger::logger(uint8_t *buffer, size_t size) : + m_buffer(buffer, size) { memset(&m_levels, 0, sizeof(m_levels)); s_log = this; @@ -64,20 +57,13 @@ logger::output(level severity, logs area, const char *fmt, va_list args) header->bytes = sizeof(entry); header->area = area; header->severity = severity; - header->sequence = m_sequence++; - size_t mlen = vsnprintf(header->message, sizeof(buffer) - sizeof(entry) - 1, fmt, args); + size_t mlen = util::vformat({header->message, sizeof(buffer) - sizeof(entry) - 1}, fmt, args); header->message[mlen] = 0; header->bytes += mlen + 1; util::scoped_lock lock {m_lock}; - if (m_immediate) { - buffer[header->bytes] = 0; - m_immediate(area, severity, header->message); - return; - } - uint8_t *out; size_t n = m_buffer.reserve(header->bytes, reinterpret_cast(&out)); if (n < header->bytes) { @@ -94,13 +80,14 @@ logger::output(level severity, logs area, const char *fmt, va_list args) size_t logger::get_entry(void *buffer, size_t size) { - util::scoped_lock lock {m_lock}; void *out; size_t out_size = m_buffer.get_block(&out); if (out_size == 0 || out == 0) { + lock.release(); m_event.wait(); + lock.reacquire(); out_size = m_buffer.get_block(&out); if (out_size == 0 || out == 0) @@ -155,5 +142,5 @@ void fatal(logs area, const char *fmt, ...) void logger_init() { - new (&g_logger) log::logger(log_buffer, sizeof(log_buffer), nullptr); + new (&g_logger) log::logger(log_buffer, sizeof(log_buffer)); } diff --git a/src/kernel/logger.h b/src/kernel/logger.h index 4f22c70..0fe4fce 100644 --- a/src/kernel/logger.h +++ b/src/kernel/logger.h @@ -29,24 +29,13 @@ constexpr unsigned areas_count = class logger { public: - /// Callback type for immediate-mode logging - typedef void (*immediate_cb)(logs, level, const char *); - /// Default constructor. Creates a logger without a backing store. - /// \arg output Immediate-mode logging output function - logger(immediate_cb output = nullptr); + logger(); /// Constructor. Logs are written to the given buffer. /// \arg buffer Buffer to which logs are written /// \arg size Size of `buffer`, in bytes - /// \arg output Immediate-mode logging output function - logger(uint8_t *buffer, size_t size, immediate_cb output = nullptr); - - /// Get the current immediate-mode callback - inline immediate_cb get_immediate() const { return m_immediate; } - - /// Register an immediate-mode log callback - inline void set_immediate(immediate_cb cb) { m_immediate = cb; } + logger(uint8_t *buffer, size_t size); /// Get the default logger. inline logger & get() { return *s_log; } @@ -78,7 +67,6 @@ public: uint8_t bytes; logs area; level severity; - uint8_t sequence; char message[0]; }; @@ -112,9 +100,6 @@ private: obj::event m_event; level m_levels[areas_count]; - immediate_cb m_immediate; - - uint8_t m_sequence; util::bip_buffer m_buffer; util::spinlock m_lock; diff --git a/src/libraries/util/format.cpp b/src/libraries/util/format.cpp new file mode 100644 index 0000000..3161a32 --- /dev/null +++ b/src/libraries/util/format.cpp @@ -0,0 +1,133 @@ +#include +#include + +namespace util { + +namespace { + +const char digits[] = "0123456789abcdef"; + +template void +append_int(char *&out, size_t &count, size_t max, I value, unsigned min_width, char pad) +{ + static constexpr size_t bufsize = sizeof(I)*3; + + unsigned num_digits = 0; + char buffer[bufsize]; + char *p = buffer + (bufsize - 1); + do { + if (value) { + *p-- = digits[value % N]; + value /= N; + } else { + *p-- = num_digits ? pad : '0'; + } + num_digits++; + } while (value || num_digits < min_width); + + ++p; + for (unsigned i = 0; i < num_digits; ++i) { + *out++ = p[i]; + if (++count == max) + return; + } +} + +void +append_string(char *&out, size_t &count, size_t max, char const *value) +{ + while (value && *value && count < max) { + count++; + *out++ = *value++; + } +} + +} // namespace + +size_t +vformat(stringbuf output, char const *format, va_list va) +{ + char * out = output.pointer; + const size_t max = output.count - 1; + + size_t count = 0; + while (format && *format && count < max) { + if (*format != '%') { + count++; + *out++ = *format++; + continue; + } + + format++; // chomp the % character + char spec = *format++; + + bool long_type = false; + if (spec == '%') { + count++; + *out++ = '%'; + continue; + } + + unsigned width = 0; + char pad = ' '; + + while (spec) { + bool done = false; + + if (spec >= '0' && spec <= '9') { + if (spec == '0' && width == 0) + pad = '0'; + else + width = width * 10 + (spec - '0'); + + spec = *format++; + continue; + } + + switch (spec) { + case 'l': + long_type = true; + break; + + case 'x': + if (long_type) + append_int(out, count, max, va_arg(va, uint64_t), width, pad); + else + append_int(out, count, max, va_arg(va, uint32_t), width, pad); + done = true; + break; + + case 'd': + case 'u': + if (long_type) + append_int(out, count, max, va_arg(va, uint64_t), width, pad); + else + append_int(out, count, max, va_arg(va, uint32_t), width, pad); + done = true; + break; + + case 's': + append_string(out, count, max, va_arg(va, const char *)); + done = true; + break; + } + + if (done) break; + spec = *format++; + } + } + + return count; +} + +size_t +format(stringbuf output, const char *format, ...) +{ + va_list va; + va_start(va, format); + size_t result = vformat(output, format, va); + va_end(va); + return result; +} + +} //namespace util diff --git a/src/libraries/util/util.module b/src/libraries/util/util.module index 1e137eb..f11e648 100644 --- a/src/libraries/util/util.module +++ b/src/libraries/util/util.module @@ -4,6 +4,7 @@ module("util", kind = "lib", sources = [ "bip_buffer.cpp", + "format.cpp", "spinlock.cpp", ], public_headers = [ @@ -13,6 +14,7 @@ module("util", "util/counted.h", "util/deque.h", "util/enum_bitfields.h", + "util/format.h", "util/hash.h", "util/linked_list.h", "util/map.h", diff --git a/src/libraries/util/util/format.h b/src/libraries/util/util/format.h new file mode 100644 index 0000000..44ddde2 --- /dev/null +++ b/src/libraries/util/util/format.h @@ -0,0 +1,16 @@ +/// \file format.h +/// sprintf-like format functions + +#pragma once + +#include +#include + +namespace util { + +using stringbuf = counted; + +size_t format(stringbuf output, const char *format, ...); +size_t vformat(stringbuf output, const char *format, va_list va); + +} diff --git a/src/user/srv.logger/main.cpp b/src/user/srv.logger/main.cpp index b5ee4ae..db5d22f 100644 --- a/src/user/srv.logger/main.cpp +++ b/src/user/srv.logger/main.cpp @@ -25,7 +25,6 @@ struct entry uint8_t bytes; uint8_t area; uint8_t severity; - uint8_t sequence; char message[0]; };