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
wrong Printf.printf output for integers with '0' flag #6938
Comments
Comment author: @gasche Thank you for this detailed report. The 4.02.1 commit that reintroduced this corner behaviour is I didn't know that the C standard mandates this -- I think Benoît and I wrongly assumed that was a quirk of the existing implementation. We should indeed fix %ld etc. to have the same backward-compatibility behavior, as you suggest. Note that if what you want is space padding, you can get it by using an actual space padding indication with "% 47" instead of "%047": Printf.printf "% 47.27ld" 1736201459l;;
Printf.printf "% 47.27Ld" 1736201459L;;
I would agree with the comment in the patch that "%047.27d" should rather be rejected statically, because its behaviour makes little sense. |
Comment author: @dbuenzli
Just to be sure I understood well, it's not the 0 flag that should be ignored but the [width] if there is a [.precision] ? |
Comment author: @gasche No, it's almost the other way around, with a hack thrown in the middle. For floats, ".27" means to use 27 digits of precision, and "027" means to pad the number zeroes (on the left). Just using "27" (eg. %27f) just means padding with spaces (on the left). For integers and family, "%.27d" is interpreted exactly as "%027d" (pad to the left with zeroes"). But then you can also use "%047.27d", and then the "0" is ignored: it means the same as "%47.27d", that is: pad with 0 upto length 27, then with spaces upto length 47. (Note that "%47027d" could not be used, as it pads with 47027 spaces) Which means, by the way, that my first message above is slightly wrong (which means I should be just writing my thesis instead of padding format strings): to pad with spaces it suffices to write "%47.27ld", the space is unnecessary. The space means "pad positive numbers with a space, so that they have the same length as negative numbers" (the other option being "+", to pad with a + sign). Oh well. |
Comment author: acascella @dbuenzli: gasche is right. It is the '0' flag that should be ignored, not the width. @gasche: actually I do not agree with the comment you point to, because simply ignoring the '0' flag will not only solve this issue, but also remain consistent with C (and thus avoiding future issues that might be introduced with other workarounds). [edit] I think that "%47027d" is a perfectly valid format: it will print a lot of spaces and then your integer (for a total of 47027 characters). Incidentally, this behavior was noticed when trying to emulate C's printf with OCaml's and failing to pass some consistency tests: http://trust-in-soft.com/tis-interpreter/ |
Comment author: @gasche I think warning on "%047.27d" (or at least having a mode that rejects it statically, as we have today) is a good thing, because this format makes strictly less sense than the equivalent "%47.27d", so we should encourage authors to clarify their intent, and either recognize that they intended something else or use the clearer expression. That said I agree that, when we accept this format, the runtime behavior should be consistent (with C, the legacy implementation, etc.).
This seems very interesting, do you have a list of other test cases? If you felt motivated to contribute to the testsuite we have for formats (testsuite/tests/lib-{printf,scanf,scanf-2,format}), that could help spot other regressions. |
Comment author: bvaugon I agree. In fact, this problem seems to have been fixed properly I attach a patch that propagates this fix to "%ld", "%Ld" and "%nd" in |
Comment author: bvaugon They are also fixed by the patch. |
Comment author: @gasche I forgot to say: the patch looks good (of course!) and it's added to my merge list for trunk -- unless someone merges it before. |
Comment author: acascella
I am currently automatically generating test cases. Once I get good coverage I might extract some meaningful cases to contribute to the test suite. |
Comment author: @xavierleroy Fix committed in July 2015 (907305c). Marking this PR as resolved. |
Original bug ID: 6938
Reporter: acascella
Status: closed (set by @xavierleroy on 2017-02-16T14:14:43Z)
Resolution: fixed
Priority: normal
Severity: minor
Platform: x86_64
OS: GNU/Linux
OS Version: 3.13.0-57-generi
Version: 4.02.2
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: standard library
Tags: patch
Monitored by: @gasche
Bug description
Printf.printf doesn't handle correctly the '0' flag for the integer conversion specifiers 'ld' and 'Ld'. This flag should be ignored if a precision is specified (to be consistent with the C standard).
Currently (from version 4.02.0), padding zeros will be added to fill the whole print width instead of padding only to the specified precision.
The behavior is correct for the 'd' conversion specifier (but wasn't working properly in 4.02.0 though).
Steps to reproduce
OCaml version 4.03.0+dev9-2015-07-22:
Printf.printf "%047.27d" 1736201459;;
Printf.printf "%047.27Ld" 1736201459L;;
0000000000000000000000000000000000001736201459- : unit = ()
Printf.printf "%047.27ld" 1736201459l;;
00000000000000000000000000000000000001736201459- : unit = ()
Additional information
The bug was introduced in version 4.02.0 where all the int formats were broken. Partial fix for the 'd' conversion specifier was introduced in 4.02.1 but the bug is still present for the 'Ld' and 'ld' conversion specifiers.
(Everything worked fine in version 4.01.0, see output below).
Printf.printf "%047.27Ld" 1736201459L;;
Printf.printf "%047.27ld" 1736201459l;;
Printf.printf "%047.27d" 1736201459;;
Printf.printf "%047.27d" 1736201459;;
00000000000000000000000000000000000001736201459- : unit = ()
Printf.printf "%047.27Ld" 1736201459L;;
00000000000000000000000000000000000001736201459- : unit = ()
Printf.printf "%047.27ld" 1736201459l;;
00000000000000000000000000000000000001736201459- : unit = ()
Printf.printf "%047.27d" 1736201459;;
Printf.printf "%047.27Ld" 1736201459L;;
00000000000000000000000000000000000001736201459- : unit = ()
Printf.printf "%047.27ld" 1736201459l;;
00000000000000000000000000000000000001736201459- : unit = ()
Printf.printf "%047.27d" 1736201459;;
Printf.printf "%047.27Ld" 1736201459L;;
0000000000000000000000000000000000001736201459- : unit = ()
Printf.printf "%047.27ld" 1736201459l;;
00000000000000000000000000000000000001736201459- : unit = ()
File attachments
The text was updated successfully, but these errors were encountered: