Coder Social home page Coder Social logo

eyalroz / printf Goto Github PK

View Code? Open in Web Editor NEW

This project forked from mpaland/printf

375.0 375.0 49.0 816 KB

Tiny, fast(ish), self-contained, fully loaded printf, sprinf etc. implementation; particularly useful in embedded systems.

License: MIT License

C 89.15% CMake 10.85%
embedded-systems printf printf-functions self-contained snprintf sprintf sprintf-style tiny utility-library vprintf

printf's People

Contributors

bart112233 avatar cz7asm avatar damianpala avatar elipsitz avatar eyalroz avatar farrrb avatar fivdi avatar fronders avatar joshklod avatar jtlenzz avatar karlk90 avatar kkoovalsky avatar leandros avatar mickjc750 avatar mjasperse avatar mpaland avatar nandyy avatar noahp avatar phillipjohnston avatar sgoll avatar stuartgorman avatar svkampen avatar usaygo avatar vgrudenic avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

printf's Issues

Support the "%n" specifier

The C standard library printf()-family functions support a %n specifier, which takes a point to an integral type and updates it with the number of characters written so far for the format string. We do not yet support this, and while it's possible to achieve the same effect with fctprintf() and a nilpotent output function, we should implement actual support for this specifier.

Clarify behavior of "%zi"

The C99 standard is vague about what exactly one is to pass for the "%zi" format specifier - it's a "signed size_t" - but that's not properly defined. We need to be explicit about what we expect for this specifier, so as not to confuse users.

Alias preprocessor define is working?

I tried to use the alias function names define, and as best I can tell it didn't do anything. Looking at the code, it isn't clear to me how it's supposed to work. I am on the latest 27 Sep commit.

#ifdef PRINTF_ALIAS_STANDARD_FUNCTION_NAMES
# define printf_    printf_
# define sprintf_   sprintf_
# define vsprintf_  vsprintf_
# define snprintf_  snprintf_
# define vsnprintf_ vsnprintf_
# define vprintf_   vprintf_
#endif

// ... some stuff here ...

#ifdef PRINTF_ALIAS_STANDARD_FUNCTION_NAMES
# undef printf_
# undef sprintf_
# undef vsprintf_
# undef snprintf_
# undef vsnprintf_
# undef vprintf_
#endif

I temporarily hand-edited my local printf.h file and went back to way function name aliasing worked in the previous 17 Aug commit. This works, and I understand why it works (unlike the current commit).

#ifdef PRINTF_ALIAS_STANDARD_FUNCTION_NAMES
# define printf    printf_
# define sprintf   sprintf_
# define vsprintf  vsprintf_
# define snprintf  snprintf_
# define vsnprintf vsnprintf_
# define vprintf   vprintf_
#endif

// ... some stuff here ...

I did read the issue #14 notes, but they didn't explain what I'm experiencing. Regarding the 27 Sep commit, am I missing something about how to make it work and/or understanding how it should work?

Thanks,
-Chris

Suggestion - renaming, avoid excessive underscores

The use of underscores in the source is excessive (IMHO).

As 'prn' is a common abbreviation of 'print', I suggest renaming printf to prnf.
This would avoid the need for most underscores, which are currently used to avoid name collisions with the standard library.
It would make it obvious in applications which use prnf(), that they are not using the standard implementation.
printf() could still be overridden by the prnf() implementation, using #defines as it is now.

sizeof(double) is not guaranteed to be 64bits

On AVR, and most likely some other 8-bit platforms, double is only 32bit.
This breaks %e and %g (but not %f).
Ideally, it would be nice to provide some kind of %e %g functionality when double is 32bit, perhaps with reduced precision.
At a minimum, a build error should occur if %e and or %g is enabled on these platforms.

Signed integer overflow when printing INT_MIN

On most machines, printf("%d", INT_MIN) will cause a signed integer overflow, resulting in undefined behavior.

At line 782 the absolute value is computed with (unsigned int)(value > 0 ? value : 0 - value). However when VALUE == INT_MIN on a typical twos-complement machine, then 0 - value is -INT_MIN which cannot be represented as int and so overflow occurs. -value would have the same issue. (For instance, on a typical 32-bit machine int can represent -2147483648 as 0x80000000, but cannot represent 2147483648.)

Compile printf.c with gcc -ftrapv or gcc -fsanitize=undefined and you'll get a runtime error for this example.

There is the same problem for printf("%ld", LONG_MIN) and printf("%lld", LLONG_MIN).

I think it should instead be value > 0 ? (unsigned int)value : -(unsigned int)value. Converting a negative int to unsigned effectively adds UINT_MAX (typically 2**32) and negating subtracts from UINT_MAX, yielding the correct result. Unsigned integer overflow in C is well defined. There will still be a problem on a machine where -INT_MIN doesn't fit in unsigned int but they should be rare. (You could cast up to unsigned long long which is a little better, but you will still have the same problem with -LLONG_MIN.)

By the way, these cases are not exercised in the test suite, which might be a good idea. It's a little tricky to add because we don't know the value of INT_MIN ahead of time, so we don't know what string to expect, but we also can't safely do printf("%d", -2147483648) because we could be on a machine with 16-bit int.

Need of CI server?

Hey,
I see that there is no CI server implemented at the moment, maybe this project needs one? If so, I'm willing to help you out. I have experience with the GitHub Actions (you can see my repos where I use Actions extensively MYREPO1, MYREPO2).

Don't alias with macros

If the library is "requested", via a CMake build option, to alias printf(), sprintf() etc. - Let's just do that with proper functions. Now, we can't "forward" variable argument functions except via va_list's but what we can do, and perhaps should do, is one of:

  1. Define both printf_ and printf functions, with the same source code.
  2. Use macros to choose between defining only printf_-style names or only printf-style names.

The CMake options-via-config-h is currently busted

The printf_config.h file is not properly generated - we're not using #cmakedefine the right way, and we need to do some name prefix magic in the CMakeLists.txt file.

... Also, there's a typo in the definedness check for PRINTF_SUPPORT_PTRDIFF_LENGTH_MODIFIER in printf.c.

Support use of CUDA in unit tests

We need to have our tests/CMakeFiles.txt allow for CUDA targets and their build. And probably make that optional so that this stuff doesn't get built by default for non-CUDA-users.

Last-precision-digit one-off errors with %e

We often have one-off errors on the last digit of a number. Autotest output:

fmt = "% e" value = 9.622188e+03
gnu = " 9.622188e+03"
mpa = " 9.622187e+03"

fmt = "%+e" value = -4.136612e+04
gnu = "-4.136612e+04"
mpa = "-4.136613e+04"

fmt = "%e" value = 8.438362e+04
gnu = "8.438362e+04"
mpa = "8.438363e+04"

getopt.h not available on Windows

The autotest.cpp program used getopt.h, but that's not available on Windows.

For now, let's just bundle an available FOSS getopt implementation; but we should probably be able to drop the dependence on those extra ~650 lines-of-code (@mickjc750 ?)

typo in readme

Hi,
I just noticed, there is typo in readme. Its Marco Paland. In fact, it's there several times

printf_config.h generation busted

We currently use #cmakedefine in our printf_config.h.in file, but printf.c expects a different logic: It expects the value to always be defined, otherwise it chooses the default value (rather than interpreting undefined as false).

Drop global constants when compiling for CUDA devices

Unfortunately (?), CUDA does not support global device-side constant data. And we happen to be using such data in our printf implementation: The powers-of-10 table. For now, let's apply a quick-and-perhaps-dirty fix: Make it a static __constant__ array inside a function for obtaining the power-of-10. Of course, for regular CPU execution, things basically stays the same (the function will just lookup a value in the global array, meaning it should be inline.

Build warning regarding sprint_exponential_number() when %ll and %f are enabled without %e

The following build warning occurs with:
PRINTF_SUPPORT_FLOAT_SPECIFIERS ON
PRINTF_SUPPORT_EXPONENTIAL_SPECIFIERS OFF
PRINTF_SUPPORT_LONG_LONG ON

Built with arm-none-eabi-gcc (15:7-2018-q2-6) 7.3.1 20180622 (release) [ARM/embedded-7-branch revision 261907]

app/src/printf.c: In function 'sprint_floating_point':
app/src/printf.c:779:10: warning: implicit declaration of function 'sprint_exponential_number'; did you mean 'sprint_decimal_number'? [-Wimplicit-function-declaration]
sprint_exponential_number(out, buffer, idx, maxlen, value, precision, width, flags, buf, len) :
^~~~~~~~~~~~~~~~~~~~~~~~~
sprint_decimal_number

Runtime error trapping for disabled type specifiers

Example:
%f is not available due to disabling PRINTF_SUPPORT_FLOAT_SPECIFIERS, and the application encounters a %f in a format string.
It would be useful to trap this so that the application can cause a runtime fault with an assertion handler.
This could be done with a macro which is ((void)0) by default, but could be edited in the config file to be something like ASSERT(false)

test suite missing DISABLE_WARNING_PRINTF_FORMAT_OVERFLOW for MSVC

This definition is missing:

#define DISABLE_WARNING_PRINTF_FORMAT_EXTRA_ARGS
#define DISABLE_WARNING_PRINTF_FORMAT_OVERFLOW

for the _MSC_VER here:

#if defined(_MSC_VER)
#define DISABLE_WARNING_PUSH __pragma(warning( push ))
#define DISABLE_WARNING_POP __pragma(warning( pop ))
#define DISABLE_WARNING(warningNumber) __pragma(warning( disable : warningNumber ))
// TODO: find the right warning number for this
#define DISABLE_WARNING_PRINTF_FORMAT
#define DISABLE_WARNING_PRINTF_FORMAT_EXTRA_ARGS

Build warning when compiled with avr-gcc (GCC) 5.4.0

printf.c: In function ‘sprint_exponential_number’:
printf.c:491:25: warning: ‘normalization.raw_factor’ may be used uninitialized in this function [-Wmaybe-uninitialized]
struct scaling_factor account_for_precision = update_normalization(normalization, prec_power_of_10);
^
printf.c:625:25: note: ‘normalization.raw_factor’ was declared here
struct scaling_factor normalization;

Support CUDA host-side execution

Allow the printf library to be used in host-side CUDA code (built with nvcc).

This probably doesn't need much real work, but - some warning-avoidance may be necessary, plus a test suite (maybe just replicate the existing one for starters?)

Pointer format %p fails (Win32)

The value is taken from va_arg initially here:

printf/printf.c

Line 1058 in bd89cc7

uintptr_t value = (uintptr_t)va_arg(va, void*);

When sizeof(uintptr_t) != sizeof(long long) (i.e. Win32) it tries to read va_arg again:

printf/printf.c

Line 1071 in bd89cc7

idx = print_integer(out, buffer, idx, maxlen, (PRINTF_INTEGER_VALUE_TYPE)((uintptr_t)va_arg(va, void*)), false, BASE_HEX, precision, width, flags);

which is wrong. The case when is_ll is true does it right:

printf/printf.c

Line 1067 in bd89cc7

idx = print_integer(out, buffer, idx, maxlen, (PRINTF_INTEGER_VALUE_TYPE) value, false, BASE_HEX, precision, width, flags);

If the else case is corrected, it would do the same. Then this conditional compilation is not necessary:
#if PRINTF_SUPPORT_LONG_LONG

printf/printf.c

Lines 1055 to 1078 in bd89cc7

case 'p' : {
width = sizeof(void*) * 2U + 2; // 2 hex chars per byte + the "0x" prefix
flags |= FLAGS_ZEROPAD | FLAGS_POINTER;
uintptr_t value = (uintptr_t)va_arg(va, void*);
if (value == (uintptr_t) NULL) {
idx = _out_rev(out, buffer, idx, maxlen, ")lin(", 5, width, flags);
}
else {
#if PRINTF_SUPPORT_LONG_LONG
const bool is_ll = sizeof(uintptr_t) == sizeof(long long);
if (is_ll) {
idx = print_integer(out, buffer, idx, maxlen, (PRINTF_INTEGER_VALUE_TYPE) value, false, BASE_HEX, precision, width, flags);
}
else {
#endif
idx = print_integer(out, buffer, idx, maxlen, (PRINTF_INTEGER_VALUE_TYPE)((uintptr_t)va_arg(va, void*)), false, BASE_HEX, precision, width, flags);
#if PRINTF_SUPPORT_LONG_LONG
}
#endif
}
format++;
break;
}

Compiling with MSVC (VS2017) this issue makes this test cases fail:
Screenshot 2021-09-21 at 17 49 03

Shouldn't check for 10e+NN

The test suite currently has some checks for expected results of "10e+2", "10e+5" etc. ; they're not exposed on my current system because they depend on the threshold of switching from decimal to exponential notation, but they're wrong.

Support floating-point printing using the "%a" specifier

The C standard library printf()-family functions support an %a specifier, which takes a double value and prints it with perfect accuracy in exponent notation with hexadecimal digits. We should support this (although with an option to not-compile this support, like we have for other floating-point capabilities.

CMake improvements for local build (with static library)

Hello! Thank you, @eyalroz, for continuing the project.

I prefer the fork, since it supports CMake. Likewise, I use it with FetchContent:

function(ProvidePrintfLibrary)

    include(FetchContent)

    set(BUILD_STATIC_LIBRARY ON)
    set(SUPPORT_EXPONENTIAL_SPECIFIERS OFF)
    set(SUPPORT_LONG_LONG OFF)
    set(ALIAS_STANDARD_FUNCTION_NAMES OFF)
    set(DEFAULT_FLOAT_PRECISION "3")
    set(MAX_INTEGRAL_DIGITS_FOR_DECIMAL "3")

    FetchContent_Declare(printf_library
        GIT_REPOSITORY https://github.com/eyalroz/printf.git
        GIT_TAG v5.0.0
    )
    FetchContent_MakeAvailable(printf_library)

    FetchContent_GetProperties(printf_library SOURCE_DIR printf_source_dir)
    target_include_directories(printf PUBLIC $<BUILD_INTERFACE:${printf_source_dir}>)
    
endfunction()

I would like to address some miscellaneous issues which I had to bypass:

  1. I use the function here to set the local variables to not mutate the parent scope. "Normal variables" are used instead of options. There might be a risk that the name of BUILD_STATIC_LIBRARY may clash with other libraries/dependencies - it's quite general. I would prefer to make the name more specific by adding a PRINTF_ prefix to the options, so the variables could be preset by the user also with the option command.
  2. I had to propagate BUILD_INTERFACE include directory, because using #include "printf.h" causes a compile error, since no include directories are propagated by the library.
  3. Some variables in the config mismatch the names used in the code. One example is PRINTF_INTEGER_BUFFER_SIZE CMake option. In the code, it is PRINT_INTEGER_BUFFER_SIZE. It means that the change to the former doesn't affect the latter.

I will create a PR that addresses the issue.

ALIAS_STANDARD_FUNCTION_NAMES doesn't work correctly

When setting the option ALIAS_STANDARD_FUNCTION_NAMES to ON only the definitions inside printf.c will be affected. Inside printf.c https://github.com/eyalroz/printf/blob/develop/src/printf/printf.c#L54:

#if PRINTF_ALIAS_STANDARD_FUNCTION_NAMES
# define printf_    printf
# define sprintf_   sprintf
# define vsprintf_  vsprintf
# define snprintf_  snprintf
# define vsnprintf_ vsnprintf
# define vprintf_   vprintf
#endif

But in the printf.h there is no include "printf_config.h", thus the declarations will never be aliased.

It means that, if the option is set to ON and the library is compiled, installed and consumed:

#include "printf.h"

int main()
{
    char c[64]; 
    snprintf(c, sizeof(c), "%d %f", 5, 160.1);

    return 0;
}

One gets compilation error:

main.cpp: In function ‘int main()’:
main.cpp:6:12: error: ‘snprintf’ was not declared in this scope
6 | snprintf(c, sizeof(c), "%d %f", 5, 160.1);
| ^~~~~~~~

Possible solutions:

  1. Install printf_config.h along with printf.h and put #include "printf.h" at the top of printf.h.
  2. Generate printf.h and rename the function names basing on the ALIAS_STANDARD_FUNCTION_NAMES option value.
  3. Generate printf.h by putting PRINTF_ALIAS_STANDARD_FUNCTION_NAMES macro dynamically at the top of the printf.h file during installation and use target_compile_definitions(printf PUBLIC PRINTF_ALIAS_STANDARD_FUNCTION_NAMES) for the build interface.
  4. Other?

Should I move printf.c and printf.h into a subdirectory?

The repository root has a bunch of non-source files in addition to printf.c and printf.h; this is slightly distracting and messy. In larger repositories, one typically places the sources files in (one or more) subdirectories, and none of them in the repository root.

Should we, then, create a src/source subdirectory, and movie printf.h and printf.c into it?

Compiler definitions issues

There are a few issues with the compiler definitions:

  • Some names I've chosen don't exactly fit the definition's actual semantics.
  • I really dislike having a pair of definitions for enabling and disabling support for things ... so I think I'll switch to a boolean value which can be defined false (or rather 0) if necessary
  • The CMakeLists.txt and printf.c explanatory comments should correspond better.
  • The CMake option names and compiler definitions should correspond better.

... and this all will be a first phase to switching to a generated configuration file for building the library.

Use a number of digits instead of an arbitrary threshold

The motivation for switching from decimal to exponential notation beyond a certain value (and breaking the standard's requirements) is the need to avoid overflowing the buffers. Therefore, the limit/threshold value should be stated in terms of buffer space, or in other words - decimal digits, each of which take up a character in the buffer.

So, let's switch to a number of digits.

Don't always alias

At the moment, the library always aliases printf, sprintf, vsprintf etc. This is problematic, because you might want to use it alongside the standard library's <stdio.h> (whether because you're using the standard library as well, or because you're including another header which itself pulls <stdio.h> without you really needing it).

So, we should have a CMake option controlling whether or not this is done.

Support fine-grained visibility in printf.c

By default, the API (defined in the README) is visible out of printf.c (and exposed in printf.h), and auxiliary functions are invisible (static). However, it is conceivable that users of this library may want:

  • Everything static - for the case of #include "printf.c".
  • the default described above
  • Everything visible (extern) - when writing printf-ish code which can stand to use our innards (for some reason...)

we could support this, e.g. similarly to how nanoprintf does.

ssize_t not available on Windows

Related to #46 .

At the moment, we pass an ssize_t to "%zi" when testing. But... there is no ssize_t on Windows, per se. Let's try to scrape one from somewhere.

When a check fails, print the arguments of the failing library fumction calls

At the moment, we use many CHECK()s in our test suite. When a CHECK() fails, we get the string literal against with the test string was compared, but we don't know what's in that buffer compared against it, nor do we know which printing function had been invoked to generate it and with which arguments.

We should do something so that the additional information gets printed.

Test suite implicitly assumes long long support

If we disable support for long long specifiers (e.g. "%llu"), a bunch of test suite checks fail. These should mostly be removed, with one or a few checks for unsupported specifier being properly ignored.

This also relates to #45 , since some of the checks may involve pointers, and there we might actually have a correctness issue when long long support is turned off.

removal of out_fct_wrap_type

This wrapper exists, as a means of passing a function pointer and a null pointer, from the interface function all the way down to the character output function, which itself is a function pointer based on the output mode. This is needed to provide fctprintf().

A better approach, is to instantiate a variable set (struct) particular to the output mode in the interface function. Then pass this by reference through to the character output function.
This makes the character output function the same format (void (out)(char, void)) as that used by fctprintf().
This removes the need for a double function pointer de-reference per character which is a performance boost.
It also removes the need to pass the 'maxlen' parameter to most of the functions, where for most output modes it is unused.

question re: my compiler tuple supports 32b ints only . . . out of luck?

Looking over the code, I noted that there is apparent support of 32b doubles (via DOUBLE_SIZE_IN_BITS and friends), and avoid making use of (u)int64_t in that case. But: my compiler supports only 32b ints (as well as only 32b doubles). Does that mean I am out of luck if I want to define PRINTF_SUPPORT_DECIMAL_SPECIFIERS
or PRINTF_SUPPORT_EXPONENTIAL_SPECIFIERS? The code makes use of int_fast64_t, which size I can't do. Has this issue been discussed? (I'd mark this as a question but do not know how)

PRINTF_INTEGER_VALUE_TYPE is problematic

PRINTF_INTEGER_VALUE_TYPE... what was I thinking?

  1. It's a macro instead of a typedef.
  2. It's an unsigned type calling itself an "integer type" (ok, only slightly confusing).
  3. We cast to it in PRINTF_ABS() ... and then apply a minus. The fact that this hasn't failed my implementation is not much than a happy fluke... :-(

Use less magic numbers

I don't like magic numbers. And while it's more difficult to get rid of them in lower-level code, and in C rather than C++ - we should still make an effort.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.