From 84319ac0b29e00066f8801623fc4566177fe0319 Mon Sep 17 00:00:00 2001 From: Alex Kotov Date: Thu, 1 Dec 2022 23:42:43 +0400 Subject: [PATCH] Maintenance (#114) --- .github/workflows/main.yml | 9 ++----- CONTRIBUTING.md | 7 ++++++ ChangeLog | 8 ++++++ Makefile.am | 1 - README.md | 2 +- bindings/mruby/src/main.c | 4 +-- common/printf.yml | 38 ++++++++++++++++++++++++++++- examples/assert.c | 3 +++ examples/generic_malloc.c | 14 ++++++----- examples/generic_mutex.c | 9 ++++--- examples/macro_container_of.c | 2 ++ examples/multiboot2_header_macro.c | 6 +++++ examples/printf_file.c | 2 +- examples/printf_file_va.c | 2 +- include/kernaux/drivers/shutdown.h | 5 ++-- include/kernaux/multiboot2.h.in | 10 +++++--- src/libc.h | 18 -------------- src/ntoa.c | 4 ++- src/pfa.c | 2 +- src/printf.c | 39 +++++++++++++++++++++--------- src/printf_fmt.c | 4 +-- src/units.c | 2 +- tests/printf_gen.jinja | 6 ++++- tests/test_memmap.c | 1 + tests/test_ntoa_assert.c | 4 +++ 25 files changed, 138 insertions(+), 64 deletions(-) delete mode 100644 src/libc.h diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index dd76a60a..09bff5a0 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -137,10 +137,5 @@ jobs: - uses: actions/checkout@v2 - name: dependencies run: sudo apt-get --yes install cppcheck - - name: cppcheck source code - # TODO: don't suppress "src/printf.c" - run: cppcheck --quiet --error-exitcode=1 --std=c99 --enable=warning,style,performance,portability --suppress='*:src/printf.c' include/ src/ libc/ - - name: cppcheck examples - run: cppcheck --quiet --error-exitcode=1 --std=c99 --enable=warning,style,performance,portability --suppress=duplicateExpression --suppress=staticStringCompare examples/ - - name: cppcheck tests - run: cppcheck --quiet --error-exitcode=1 --std=c99 --enable=warning,style,performance,portability --suppress=unusedStructMember tests/ + - name: cppcheck + run: cppcheck --quiet --error-exitcode=1 --std=c99 --enable=warning,style,performance,portability --inline-suppr examples/ include/ libc/ src/ tests/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 11caaabc..5b038900 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,6 +62,13 @@ Avoid stupid errors with: * Default case in switch statements * Braces (curly brackets) around code blocks +### Things to review periodically + +* `git grep -i fixme` +* `git grep -i todo` +* `git grep -i cppcheck-suppress` +* `git grep -i rubocop:disable` + C language diff --git a/ChangeLog b/ChangeLog index ed4c0669..bf6bb203 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2022-12-01 Alex Kotov + + * src/printf.c: Fix a bug with too big float precision + 2022-11-30 Alex Kotov * configure.ac: Fix CFLAGS setting @@ -10,6 +14,10 @@ "KERNAUX_RETURNS_TWICE", "KERNAUX_ASM" * include/kernaux/printf_fmt.h: Make stable +2022-11-27 Alex Kotov + + * src/free_list.c: Bug fixed + 2022-11-26 Alex Kotov libkernaux 0.5.0 released diff --git a/Makefile.am b/Makefile.am index e25d75e2..b7d28570 100644 --- a/Makefile.am +++ b/Makefile.am @@ -30,7 +30,6 @@ lib_LTLIBRARIES = libkernaux.la libkernaux_la_LIBADD = libkernaux_la_SOURCES = \ src/assert.c \ - src/libc.h \ src/generic/malloc.c \ src/generic/mutex.c diff --git a/README.md b/README.md index 469571c5..d930fd07 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ API ### Headers We use [semantic versioning](https://semver.org) for stable APIs. Stable APIs -can only change when major version number is increased (or minor while major is +may only change when major version number is increased (or minor while major is zero). Work-in-progress APIs can change at any time. * Basic features diff --git a/bindings/mruby/src/main.c b/bindings/mruby/src/main.c index 0e3142f7..d47e165f 100644 --- a/bindings/mruby/src/main.c +++ b/bindings/mruby/src/main.c @@ -37,7 +37,7 @@ void mrb_mruby_kernaux_gem_init(mrb_state *const mrb) #endif // KERNAUX_VERSION_WITH_PRINTF } -void current_mrb_start(mrb_state *mrb) +void current_mrb_start(mrb_state *const mrb) { mrb_assert(mrb_stack_count < MRB_STACK_SIZE - 1); mrb_assert(mrb_stack[mrb_stack_count] == NULL); @@ -46,7 +46,7 @@ void current_mrb_start(mrb_state *mrb) mrb_stack[mrb_stack_count++] = mrb; } -void current_mrb_finish(mrb_state *mrb) +void current_mrb_finish(mrb_state *const mrb) { mrb_assert(mrb_stack_count > 0); mrb_assert(mrb_stack[mrb_stack_count - 1] != NULL); diff --git a/common/printf.yml b/common/printf.yml index a8cc199a..490d7bee 100644 --- a/common/printf.yml +++ b/common/printf.yml @@ -89,7 +89,43 @@ - result: '1.200000' args: [['%f', 1.2]] float: true - - result: '123.456789' args: [['%f', 123.456789]] float: true + +- result: '0.01234568' + args: [['%.8f', 0.0123456789012345678901234567890123456789]] + float: true +- result: '0.012345679' + args: [['%.9f', 0.0123456789012345678901234567890123456789]] + float: true +# Actual precision is no more than 9 +- result: '0.0123456790' + args: [['%.10f', 0.0123456789012345678901234567890123456789]] + float: true +- result: '0.01234567900' + args: [['%.11f', 0.0123456789012345678901234567890123456789]] + float: true +- result: '0.012345679000' + args: [['%.12f', 0.0123456789012345678901234567890123456789]] + float: true +- result: '0.012345679000000000000000000000' + args: [['%.30f', 0.0123456789012345678901234567890123456789]] + float: true +# Actual length is no more than 32 +- result: '0.012345679000000000000000000000' + args: [['%.31f', 0.0123456789012345678901234567890123456789]] + float: true +- result: '0.012345679000000000000000000000' + args: [['%.32f', 0.0123456789012345678901234567890123456789]] + float: true +# Actual length is no more than 32 +- result: '10.01234567900000000000000000000' + args: [['%.32f', 10.0123456789012345678901234567890123456789]] + float: true +- result: '100.0123456790000000000000000000' + args: [['%.32f', 100.0123456789012345678901234567890123456789]] + float: true +- result: '1000.012345679000000000000000000' + args: [['%.32f', 1000.0123456789012345678901234567890123456789]] + float: true diff --git a/examples/assert.c b/examples/assert.c index 285116da..d9cdaf39 100644 --- a/examples/assert.c +++ b/examples/assert.c @@ -25,6 +25,7 @@ int main() { kernaux_assert_cb = assert_cb; + // cppcheck-suppress duplicateExpression KERNAUX_ASSERT(1 == 1); assert(count == 0); @@ -32,6 +33,7 @@ int main() assert(last_line == 0); assert(last_str == NULL); + // cppcheck-suppress duplicateExpression KERNAUX_ASSERT(1 != 1); assert(count == 1); @@ -39,6 +41,7 @@ int main() assert(last_line == __LINE__ - 4); assert(strcmp(last_str, "1 != 1") == 0); + // cppcheck-suppress staticStringCompare KERNAUX_ASSERT(strcmp("qwe", "rty") == 0); assert(count == 2); diff --git a/examples/generic_malloc.c b/examples/generic_malloc.c index cda73632..839d78dc 100644 --- a/examples/generic_malloc.c +++ b/examples/generic_malloc.c @@ -35,12 +35,14 @@ static void *MyMalloc_realloc(void *malloc, void *ptr, size_t size); struct MyMalloc MyMalloc_create() { - struct MyMalloc my_malloc; - my_malloc.malloc.calloc = MyMalloc_calloc; - my_malloc.malloc.free = MyMalloc_free; - my_malloc.malloc.malloc = MyMalloc_malloc; - my_malloc.malloc.realloc = MyMalloc_realloc; - return my_malloc; + return (struct MyMalloc){ + .malloc = { + .calloc = MyMalloc_calloc, + .free = MyMalloc_free, + .malloc = MyMalloc_malloc, + .realloc = MyMalloc_realloc, + }, + }; } void *MyMalloc_calloc(void *const malloc, const size_t nmemb, const size_t size) diff --git a/examples/generic_mutex.c b/examples/generic_mutex.c index af7199b4..f728781e 100644 --- a/examples/generic_mutex.c +++ b/examples/generic_mutex.c @@ -35,9 +35,12 @@ static void MyMutex_unlock(void *mutex); struct MyMutex MyMutex_create() { - struct MyMutex my_mutex; - my_mutex.mutex.lock = MyMutex_lock; - my_mutex.mutex.unlock = MyMutex_unlock; + struct MyMutex my_mutex = { + .mutex = { + .lock = MyMutex_lock, + .unlock = MyMutex_unlock, + }, + }; if (pthread_mutex_init(&my_mutex.pthread_mutex, NULL) != 0) abort(); return my_mutex; } diff --git a/examples/macro_container_of.c b/examples/macro_container_of.c index 3c92f3eb..397e76f6 100644 --- a/examples/macro_container_of.c +++ b/examples/macro_container_of.c @@ -1,6 +1,7 @@ #include #include +#include struct Foo { const char *hello; @@ -29,5 +30,6 @@ void example_main() assert(bar_ptr->a == 143); assert(bar_ptr->b == 820794098); + assert(strcmp(bar_ptr->foo.hello, "Hello, World!") == 0); assert(bar_ptr->c == 10981); } diff --git a/examples/multiboot2_header_macro.c b/examples/multiboot2_header_macro.c index 57e2542d..1cf5faac 100644 --- a/examples/multiboot2_header_macro.c +++ b/examples/multiboot2_header_macro.c @@ -11,6 +11,8 @@ static const struct { // of type "KernAux_Multiboot2_HTag_InfoReq" // when the number of requested information // tag types is even (n % 2 == 0). + // + // cppcheck-suppress unknownMacro KERNAUX_MULTIBOOT2_HFIELDS_INFO_REQ_EVEN( // This is the name of the structure field. tag_info_req_even, @@ -22,6 +24,8 @@ static const struct { // of type "KernAux_Multiboot2_HTag_InfoReq" // when the number of requested information // tag types is odd (n % 2 == 1). + // + // cppcheck-suppress unknownMacro KERNAUX_MULTIBOOT2_HFIELDS_INFO_REQ_ODD( // This is the name of the structure field. tag_info_req_odd, @@ -34,6 +38,8 @@ static const struct { _align1 ) // This macro may be used for all other header tag types. + // + // cppcheck-suppress unknownMacro KERNAUX_MULTIBOOT2_HFIELDS_COMMON( // This is the name of the structure field. tag_none, diff --git a/examples/printf_file.c b/examples/printf_file.c index 1b5f8bca..0cef182a 100644 --- a/examples/printf_file.c +++ b/examples/printf_file.c @@ -12,7 +12,7 @@ static const char *const data = "foobar"; static char buffer[BUFFER_SIZE]; static size_t buffer_index = 0; -static void my_putchar(const char chr, void *arg) +static void my_putchar(const char chr, void *const arg) { assert(arg == data); if (buffer_index >= BUFFER_SIZE) abort(); diff --git a/examples/printf_file_va.c b/examples/printf_file_va.c index 75562bca..d8dd5c6c 100644 --- a/examples/printf_file_va.c +++ b/examples/printf_file_va.c @@ -12,7 +12,7 @@ static const char *const data = "foobar"; static char buffer[BUFFER_SIZE]; static size_t buffer_index = 0; -static void my_putchar(const char chr, void *arg) +static void my_putchar(const char chr, void *const arg) { assert(arg == data); if (buffer_index >= BUFFER_SIZE) abort(); diff --git a/include/kernaux/drivers/shutdown.h b/include/kernaux/drivers/shutdown.h index 4be02ddb..b3d83470 100644 --- a/include/kernaux/drivers/shutdown.h +++ b/include/kernaux/drivers/shutdown.h @@ -7,9 +7,8 @@ extern "C" { #include -KERNAUX_NORETURN -void kernaux_drivers_shutdown_halt(); -void kernaux_drivers_shutdown_poweroff(); +KERNAUX_NORETURN void kernaux_drivers_shutdown_halt(); +KERNAUX_NORETURN void kernaux_drivers_shutdown_poweroff(); #ifdef __cplusplus } diff --git a/include/kernaux/multiboot2.h.in b/include/kernaux/multiboot2.h.in index b2b8f146..5b1d94bb 100644 --- a/include/kernaux/multiboot2.h.in +++ b/include/kernaux/multiboot2.h.in @@ -15,7 +15,13 @@ extern "C" { #define KERNAUX_MULTIBOOT2_HEADER_MAGIC 0xe85250d6 #define KERNAUX_MULTIBOOT2_INFO_MAGIC 0x36d76289 -#define KERNAUX_MULTIBOOT2_HEADER_ALIGN 4 +// @see https://www.gnu.org/software/grub/manual/multiboot2/multiboot.html#OS-image-format +#define KERNAUX_MULTIBOOT2_HEADER_ALIGN 8 +// @see https://www.gnu.org/software/grub/manual/multiboot2/multiboot.html#Basic-tags-structure +#define KERNAUX_MULTIBOOT2_INFO_ALIGN 8 +// @see https://www.gnu.org/software/grub/manual/multiboot2/multiboot.html#Header-tags +// @see https://www.gnu.org/software/grub/manual/multiboot2/multiboot.html#Basic-tags-structure +#define KERNAUX_MULTIBOOT2_TAG_ALIGN 8 #define KERNAUX_MULTIBOOT2_HEADER_CHECKSUM(arch, total_size) \ ((uint32_t)(-( \ @@ -26,8 +32,6 @@ extern "C" { #define KERNAUX_MULTIBOOT2_DATA(ptr) (((uint8_t*)(ptr)) + sizeof(*(ptr))) -#define KERNAUX_MULTIBOOT2_TAG_ALIGN 8 - #define KERNAUX_MULTIBOOT2_HTAG_NEXT(tag_base) \ ((struct KernAux_Multiboot2_HTagBase*)KERNAUX_MULTIBOOT2_TAG_NEXT(tag_base)) #define KERNAUX_MULTIBOOT2_ITAG_NEXT(tag_base) \ diff --git a/src/libc.h b/src/libc.h deleted file mode 100644 index b7415653..00000000 --- a/src/libc.h +++ /dev/null @@ -1,18 +0,0 @@ -// TODO: remove this file, require needed headers separately - -#ifndef KERNAUX_INCLUDED_SRC_LIBC -#define KERNAUX_INCLUDED_SRC_LIBC - -#ifdef __cplusplus -extern "C" { -#endif - -#include -#include -#include - -#ifdef __cplusplus -} -#endif - -#endif diff --git a/src/ntoa.c b/src/ntoa.c index 7c8ae1d5..447da60d 100644 --- a/src/ntoa.c +++ b/src/ntoa.c @@ -47,9 +47,11 @@ char *kernaux_utoa(uint64_t value, char *buffer, int base, const char *prefix) char *pos = buffer; if (value == 0) *(pos++) = '0'; while (value > 0) { + // cppcheck-suppress zerodivcond const char mod = value % base; *(pos++) = mod < 10 ? mod + '0' : mod - 10 + alpha; - value /= base; + // cppcheck-suppress zerodivcond + value /= base; } char *const result = pos; *(pos--) = '\0'; diff --git a/src/pfa.c b/src/pfa.c index 4534a072..e54e59ba 100644 --- a/src/pfa.c +++ b/src/pfa.c @@ -6,7 +6,7 @@ #include #include -#include "libc.h" +#include #define PAGE_INDEX(page_addr) ((page_addr) / KERNAUX_PFA_PAGE_SIZE) diff --git a/src/printf.c b/src/printf.c index 48255e4a..2f05c33b 100644 --- a/src/printf.c +++ b/src/printf.c @@ -17,10 +17,9 @@ #include #include -#include "libc.h" - #include #include +#include // import float.h for DBL_MAX #ifdef ENABLE_FLOAT @@ -245,6 +244,7 @@ int _vsnprintf(out_fct_type out, char* buffer, const size_t maxlen, const char* case KERNAUX_PRINTF_FMT_TYPE_PTR: { const bool is_ll = sizeof(uintptr_t) == sizeof(long long); + // cppcheck-suppress knownConditionTrueFalse if (is_ll) { idx = _ntoa_long_long(out, buffer, idx, maxlen, (uintptr_t)va_arg(va, void*), false, 16u, spec.precision, spec.width, spec.flags); } else { @@ -425,10 +425,6 @@ size_t _ntoa_long_long(out_fct_type out, char* buffer, size_t idx, size_t maxlen // internal ftoa for fixed decimal floating point size_t _ftoa(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double value, unsigned int prec, unsigned int width, unsigned int flags) { - char buf[PRINTF_FTOA_BUFFER_SIZE]; - size_t len = 0u; - double diff = 0.0; - // powers of 10 static const double pow10[] = { 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000 }; @@ -457,16 +453,18 @@ size_t _ftoa(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double v if (!(flags & KERNAUX_PRINTF_FMT_FLAGS_PRECISION)) { prec = PRINTF_DEFAULT_FLOAT_PRECISION; } + + char buf[PRINTF_FTOA_BUFFER_SIZE]; + size_t len = 0u; + + const unsigned int orig_prec = prec; // limit precision to 9, cause a prec >= 10 can lead to overflow errors - while ((len < PRINTF_FTOA_BUFFER_SIZE) && (prec > 9u)) { - buf[len++] = '0'; - prec--; - } + if (prec > 9u) prec = 9u; int whole = (int)value; double tmp = (value - whole) * pow10[prec]; unsigned long frac = (unsigned long)tmp; - diff = tmp - frac; + double diff = tmp - frac; if (diff > 0.5) { ++frac; @@ -484,6 +482,7 @@ size_t _ftoa(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double v if (prec == 0u) { diff = value - (double)whole; + // cppcheck-suppress redundantCondition if ((!(diff < 0.5) || (diff > 0.5)) && (whole & 1)) { // exactly 0.5 and ODD, then round up // 1.5 -> 2, but 2.5 -> 2 @@ -537,6 +536,24 @@ size_t _ftoa(out_fct_type out, char* buffer, size_t idx, size_t maxlen, double v } } + // This slows down the algorighm, but + // only if the precision was more than 9. + if (orig_prec > prec) { + const size_t space_left = PRINTF_FTOA_BUFFER_SIZE - len; + const size_t zeroes_wanted = orig_prec - prec; + const size_t delta = + space_left < zeroes_wanted ? space_left : zeroes_wanted; + + for (size_t rev_index = 0; rev_index < len; ++rev_index) { + const size_t index = len - 1 - rev_index; + buf[index + delta] = buf[index]; + } + + len += delta; + + for (size_t index = 0; index < delta; ++index) buf[index] = '0'; + } + return _out_rev(out, buffer, idx, maxlen, buf, len, width, flags); } diff --git a/src/printf_fmt.c b/src/printf_fmt.c index 490292dd..ea1886ea 100644 --- a/src/printf_fmt.c +++ b/src/printf_fmt.c @@ -12,11 +12,11 @@ #include #include -#include "libc.h" - +#include #include #include #include +#include typedef struct KernAux_PrintfFmt_Spec *Spec; diff --git a/src/units.c b/src/units.c index 87f04db2..67cb8d2b 100644 --- a/src/units.c +++ b/src/units.c @@ -6,7 +6,7 @@ #include #include -#include "libc.h" +#include #define TMP_BUFFER_SIZE (64) diff --git a/tests/printf_gen.jinja b/tests/printf_gen.jinja index a30db42d..47ec110b 100644 --- a/tests/printf_gen.jinja +++ b/tests/printf_gen.jinja @@ -40,6 +40,10 @@ static void test(const char *const expected, const char *const format, ...) va_start(va, format); result = kernaux_vfprintf(test_putc, (void*)data, format, va); va_end(va); + + printf("Exp: %s\n", expected); + printf("Got: %s\n\n", buffer); + assert((size_t)result == strlen(expected)); assert(strcmp(expected, buffer) == 0); @@ -48,6 +52,7 @@ static void test(const char *const expected, const char *const format, ...) va_start(va, format); result = kernaux_vsnprintf(buffer, sizeof(buffer), format, va); va_end(va); + assert((size_t)result == strlen(expected)); assert(strcmp(expected, buffer) == 0); } @@ -63,7 +68,6 @@ void test_main() {% if case.float %} #ifdef ENABLE_FLOAT {% endif %} - printf("%s\n", {{ escape_str(case.result) }}); test({{ escape_str(case.result) }}, {{ escape_str(fmt(case.args)) }}{{ values(case.args) }}); {% if case.float %} #endif diff --git a/tests/test_memmap.c b/tests/test_memmap.c index 5ec44fe4..ff35279d 100644 --- a/tests/test_memmap.c +++ b/tests/test_memmap.c @@ -37,6 +37,7 @@ static void before_assert() static void expect_assert() { #ifdef ENABLE_DEBUG + // cppcheck-suppress assignmentInAssert assert(assert_count_ctr == ++assert_count_exp); assert(strstr(assert_last_file, "src/memmap.c") != NULL); assert_last_file = NULL; diff --git a/tests/test_ntoa_assert.c b/tests/test_ntoa_assert.c index 77865545..108467b9 100644 --- a/tests/test_ntoa_assert.c +++ b/tests/test_ntoa_assert.c @@ -42,6 +42,7 @@ static void test_utoa_assert(char *const buffer, const int base) if (setjmp(jmpbuf) == 0) { kernaux_utoa(0, buffer, base, NULL); } else { + // cppcheck-suppress assignmentInAssert assert(assert_count_ctr == ++assert_count_exp); assert(strstr(assert_last_file, "src/ntoa.c") != NULL); assert(assert_last_line != 0); @@ -58,6 +59,7 @@ static void test_itoa_assert(char *const buffer, const int base) if (setjmp(jmpbuf) == 0) { kernaux_itoa(0, buffer, base, NULL); } else { + // cppcheck-suppress assignmentInAssert assert(assert_count_ctr == ++assert_count_exp); assert(strstr(assert_last_file, "src/ntoa.c") != NULL); assert(assert_last_line != 0); @@ -96,6 +98,7 @@ void test_main() kernaux_utoa(123, buffer, 'd', TOO_LONG_PREFIX); } else { assert(strcmp(buffer, VALID_LONG_PREFIX) == 0); + // cppcheck-suppress assignmentInAssert assert(assert_count_ctr == ++assert_count_exp); assert(strstr(assert_last_file, "src/ntoa.c") != NULL); assert(assert_last_line != 0); @@ -131,6 +134,7 @@ void test_main() kernaux_itoa(123, buffer, 'd', TOO_LONG_PREFIX); } else { assert(strcmp(buffer, VALID_LONG_PREFIX) == 0); + // cppcheck-suppress assignmentInAssert assert(assert_count_ctr == ++assert_count_exp); assert(strstr(assert_last_file, "src/ntoa.c") != NULL); assert(assert_last_line != 0);