mpaland/printf

Hash modifier with hexadecimal format

tikonen opened this issue · 11 comments

Hi,

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

uint16_t val = 0x1001;
printf("%#04x", val);

I fixed this in my fork (avr-progmem), commit d85de16
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.

@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).

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

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

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

should produce 01001, but produces 0001.

... 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.

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.

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!

@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.

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

@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.