Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code fix / improvements #60

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Code fix / improvements #60

wants to merge 11 commits into from

Conversation

ledvinap
Copy link

Apply some changes to existing code. Split into separate commits with comments

Handle 0x / 0b when width is equal to string size (issue mpaland#50):
printf("%#4x", 0x1234); -> "0x1234"

Do not print # prefix if it does not fit into PRINTF_NTOA_BUFFER_SIZE

%#<prec>o is implemented as in libc:
- printf("%#0o", 0); does output octal prefix ("0")

- printf("%#3o", 1); printf "001" - padding zero is used as octal
  prefix (issue mpaland#53)

Left padding and precision is handled correctly (issue mpaland#49):
- printf("%-10.6d", 1024); -> "001024    "
Fixes issue mpaland#51 - fractional part may overflow in round-to-even part

Rounding is still inexact, some numbers are rounded wrong way
printf("%#.0f", 1) -> "1."
Old code did use -308 as exponent for 0.0, now value is printed with
zero exponent
- %g precision is number of significant digits. Default %g precision
   shall be honored when printing as %f

printf("%g", 123.4567); -> "123.457"

- %g prints zero correctly (except for trailing zeroes)

printf("%g", 0); -> "0.0"

- %g shall switch to %f if supplied value fits into specified precision

printf("%.8g", 12345678); -> "12345678"
Negative value passed by "%.*" shall be taken as if no precision was
specified
uses _out_pad, removes some unnecessary tests (length of string to print
is known)
Signed overflow is undefined in C, this was causing problem when
printing INT_MIN (gcc sign-extended value first, so 2^64-INT_MIN got printed)
@codecov-io
Copy link

Codecov Report

Merging #60 into master will decrease coverage by 1.37%.
The diff coverage is 93.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
- Coverage     100%   98.62%   -1.38%     
==========================================
  Files           1        1              
  Lines         359      363       +4     
==========================================
- Hits          359      358       -1     
- Misses          0        5       +5
Impacted Files Coverage Δ
printf.c 98.62% <93.82%> (-1.38%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21eea6c...e29f0f8. Read the comment docs.

@axiomlegend
Copy link

Calling *out for each character is costly. Consider sprintf functionality. you call *out for every character only to do: if (a < b) a[i] = c; Lot of overhead for a few instructions.
Definitely need to put most options to ntoa_format, etc. into a struct vs stack.

@ledvinap
Copy link
Author

@axiomlegend :
Yes, out is quite expensive, but also generic. In a lot of embedded applications output to serial port is (or SWO) desired, and using sprintf will complicate things a lot.
One improvement is to use write semantics for output. Number of calls will be reduced, 1 write between %, 1 call for value output and one (or few chunks) for padding.
Another optimization in my list is to move buffer limit test out of out function, slightly improving performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants