Skip to content

Conversation

@nordic-krch
Copy link
Contributor

PR is based on #30675 but it extends it with support for creating packages statically. Statically means that a package (serialized string with arguments) is created as assignment to a buffer. Opposed to runtime package creation with cbprintf_package() (added by #30675) where string is analyzed and based on format specifiers a package is created.

In principle, CBPRINTF_STATIC_PACKAGE(buf, len, "test %d", n) resolves to assignments:

buf[0] = 0;
*(char **)&buf[1] = "test %d";
*(int *)&buf[5] = n;
len = 1 + sizeof(char *) + sizeof(int);

Added macros for:

  • determining if static packaging can be done
    CBPRINTF_MUST_RUNTIME_PACKAGE returns 1 if runtime packaging must be used.
  • determining size of the package
    In order to calculate length that can be assigned to const variable 2 step method is used. First part is creating local copies of arguments (handling sizeof("string") case). Second step macro returns package size:
CBPRINTF_STATIC_PACKAGE_SIZE_TOKEN(_tmp, "test %d", n);
static const size_t len = CBPRINTF_STATIC_PACKAGE_SIZE(_tmp, "test %d", n);
  • creating a package
    Creating package on stack snippet:
CBPRINTF_STATIC_PACKAGE_SIZE_TOKEN(_tmp, fmt, ...);
static const size_t len = CBPRINTF_STATIC_PACKAGE_SIZE(_tmp, fmt, ...);
uint8_t buf[len];
CBPRINTF_STATIC_PACKAGE(buf, len, fmt, ...);

Extended test suite to compare static packaging against runtime whenever possible. Additionally, added 2 tests which validates macros behavior.

I have initial version of logger which is using this extension and supports basic functionality.

pabigot and others added 15 commits December 12, 2020 15:39
If a precision flag is included for s formatting that bounds the
maximum output length, so we need to use strnlen rather than strlen to
get the amount of data to emit.  With that flag we can't expect there
to be a terminating NUL following the text to print.

Also fix handling of an empty precision, which should behave as if a
precision of zero was provided.

Signed-off-by: Peter Bigot <[email protected]>
Replace SANITYCHECK flag with TWISTER.

Signed-off-by: Peter Bigot <[email protected]>
TC_PRINTF doesn't append a newline, so the skip messages sometimes ran
into the test result message.

Signed-off-by: Peter Bigot <[email protected]>
Providing a literal width or precision that exceeds the non-negative
range of int does not appear to be rejected by the standard, but it
does produce a build diagnostic so we can't test it.  Switch to an
equivalent form that doesn't affect line coverage.

Signed-off-by: Peter Bigot <[email protected]>
This function was designed to support the logging infrastructure's
need to copy values from va_list structures.  It did not meet that
need, since some values need to be changed based on additional data
that is only available when the complete format specification is
examined.  Remove the function as unnecessary.

Signed-off-by: Peter Bigot <[email protected]>
It may not be clear that the length modifiers reference native C types
with specific ranks.  Document the core type for each modifier.

Signed-off-by: Peter Bigot <[email protected]>
The extraction of values from varargs was inlined in cbvprintf()
because va_list arguments cannot be passed into subroutines by
reference, and there was only one place that needed this
functionality.  An upcoming enhancement requires extracting the
arguments but processing them later, so outline the extraction code.

Signed-off-by: Peter Bigot <[email protected]>
Format width and precision can be extracted from arguments.  A
negative value is interpreted by changing flag state and using the
non-negative value.

Refactor pull_va_args() so the raw value from the stack is preserved
in the state so it can be packed, deferring the interpretation to
point-of-use.

Signed-off-by: Peter Bigot <[email protected]>
An upcoming enhancement supports conversion with information provided
from sources other than a va_list.  Add a level of indirection so the
core conversion operation isn't tied to the stdarg API.

Signed-off-by: Peter Bigot <[email protected]>
Several tests require additional stack space to accommodate a deeper
call stack due to inability to inline functions in an upcoming
enhancement.

Signed-off-by: Peter Bigot <[email protected]>
In applications like logging the call site where arguments to
formatting are available may not be suitable for performing the
formatting, e.g. when the output operation can sleep.  Add API that
supports capturing data that may be transient into a buffer that can
be saved, and API that then produces the output later using the
packaged arguments.

Signed-off-by: Peter Bigot <[email protected]>
Double vs long double handling was swapped in packaging. It was wrong
in encoder and decoder so error gone unnoticed.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
Use int for H and HH values. This allows to statically package all arguments
of size int and smaller.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
Using wint_t instead of unsigned long to package %lc.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
Fixed compilation issues with wint_t by replacing it with wchar_t.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
@nordic-krch nordic-krch requested a review from pabigot January 5, 2021 14:18
@github-actions github-actions bot added area: API Changes to public APIs area: Kernel area: Tests Issues related to a particular existing or missing test labels Jan 5, 2021
@nordic-krch nordic-krch mentioned this pull request Jan 22, 2021
@nordic-krch nordic-krch force-pushed the cbprintf_static_package branch 2 times, most recently from 2de98cf to f867185 Compare February 1, 2021 10:34
Added version which compiles much faster and provide similar
functionality (with few limitations). If macros are widely used
then compile time is significantly shorter (order of magnitude).

Signed-off-by: Krzysztof Chruscinski <[email protected]>
Added tests for FAST_FOR_EACH_x, FAST_GET_ARG_N and
FAST_GET_ARGS_LESS_N.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
Added macro for determining if string can be statically packaged.
Added macro for static packaging of a string.

Added cbprintf_internal.h with internal macros to avoid spoiling
API file.

Additionally, added option for packaging to store first char pointer
(fmt) as a standard pointer witouth 1 byte header which distinguish
between storing by pointer and storing by value. This can significantly
reduce storage, e.g. string without any arguments uses one word vs
two word (byte header + alignment).

Signed-off-by: Krzysztof Chruscinski <[email protected]>
Added new tests and modified existing tests to validate static packaging.
If static packaging is enabled then it is performed whenever possible.

Dedicated test is used for validating static packaging for cases where
runtime package may differ due to padding.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
@nordic-krch nordic-krch force-pushed the cbprintf_static_package branch from c2393cc to eeef7cc Compare February 4, 2021 07:53
In order to create a package each parameter must be promoted
by adding 0. However, it generates a warning for void * arguments.
Added pragma to temporarly suppress that warning inside the macro.

Signed-off-by: Krzysztof Chruscinski <[email protected]>
@nordic-krch
Copy link
Contributor Author

Superseded by #32863.

@nordic-krch nordic-krch closed this Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: API Changes to public APIs area: Kernel area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants