mpaland / printf

Tiny, fast, non-dependent and fully loaded printf implementation for embedded systems. Extensive test suite passing.
MIT License
2.47k stars 452 forks source link

Hash modifier with hexadecimal format #99

Open tikonen opened 3 years ago

tikonen commented 3 years ago

Hi,

Following format string outputs "0x01" when it should output "0x1001".

uint16_t val = 0x1001;
printf("%#04x", val);
mickjc750 commented 3 years ago

I fixed this in my fork (avr-progmem), commit d85de16af01808f4bde231614f05b7cc65cc7d26 Around line 430, I had to remove: if (!(flags & FLAGS_PRECISION) && len && ((len == prec) || (len == width))) { len--; if (len && (base == 16U)) { len--; } }

It's because 0x adds 2 characters to the length. It's probably also fixed in other pull requests here.

eyalroz commented 3 years ago

@mickjc750 : You'd be surprised; for PR's have addressed this issue. But - I'm working on it for my own fork (which aims to be general, not specific to "avr-progrmem", and incorporate the various fixes suggested over the past few years here).

eyalroz commented 3 years ago

... plus - just removing this results in test case failures. Have you verified all those test case checks are invalid?

eyalroz commented 3 years ago

Oh, I should also note this problem manifests with the %o specifier as well:

printf_("%#04o", 01001);

should produce 01001, but produces 0001.

mickjc750 commented 3 years ago

... plus - just removing this results in test case failures. Have you verified all those test case checks are invalid?

It didn't break any test cases. At least none that existed at the time.

eyalroz commented 3 years ago

It didn't break any test cases. At least none that existed at the time.

Ok, but... it breaks the "padding #020" test case on the master branch of this repository.

mickjc750 commented 3 years ago

It didn't break any test cases. At least none that existed at the time.

Ok, but... it breaks the "padding #20" test case on the master branch of this repository.

Ugh.. So it does. In fact 0 padding is broken in my fork (it over pads by 2 0's). Looks like I will need to dissect these flag acrobatics and fix it. Thankyou!

eyalroz commented 3 years ago

@mickjc750 : I want to make a suggestion. There's been a lot repeated work that different people have done over what's currently in this repository; and I've been trying to integrate it on my fork (seeing how mpaland has been inactive for two years; though this might change). Now, I realize your repository has a specific scope that's different from the general one(s) - but I still assume it should be useful to you to be able to base it off an up-to-date and responsive general repository, so that your changes remain shallower. If that's indeed the case, please consider trying to do that on the general repository, and possibly coordinating that work with me (or my fork...) so that our efforts don't diverge so much.

eyalroz commented 2 years ago

@mickjc750 : Both 0 padding and this issue are now fixed on my fork.

mickjc750 commented 2 years ago

Awesome. I think I should rebase my fork from yours, thanks!

On Sat, Jul 31, 2021 at 1:13 AM Eyal Rozenberg @.***> wrote:

@mickjc750 https://github.com/mickjc750 : Both 0 padding and this issue are now fixed on my fork.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mpaland/printf/issues/99#issuecomment-889960015, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ2OLICFF2J7NMH5KOAJ6LT2K6THANCNFSM42UNXPUQ .

--

Michael Clift - Design Engineer.

Clift Innovations Pty Ltd Suit 10/13 Walkers Road Nunawading, VIC 3131

ABN 24 286188328

Tel: +61 3 9877 3109 Fax: +61 3 9999 1406 Mob: 0417 613 680 Email: @.*** Web: www.clift.com.au


PRIVATE AND CONFIDENTIAL This e-mail and any files transmitted with it are intended solely for the use of the addressee(s) and may contain information that is confidential or privileged. If you receive this e-mail and you are not the addressee(s), please disregard the contents of the e-mail, delete the e-mail and notify the author immediately.


eyalroz commented 2 years ago

@mickjc750 : In that case, please try to separate your changes into universal ones, not specific to your target platform, which I could consider for my fork, and the purely AVR-specific changes. Perhaps even use separate branches for them.