On 07/21/2017 11:48 AM, Markus Armbruster wrote: > I forgot that PRId64 expands into non-standard crap on some systems. > > Options: > > 1. Outlaw use of PRI macros, limit integer length modifiers for > conversion specifiers "d" and "u" to "l" and "ll". When PRI macros > creep in, the build breaks on hosts where they expand to anything > else (hopefully, our CI will catch that). Interpolating int64_t > values become a bit awkward. > > Required work: fix my patches not to use PRId64, drop support for > length modifier "I64" from parse_escape().
Yuck. (But motivation for my earlier series to nuke qobject_from_jsonf()) > > 2. Support PRId64 and PRIu64, whatever their actual value may be. > > a. Support all possible values. This is what we've tried before. > Nasty: fails only at run-time on hosts with sufficiently creative > values. > > Required work: add support for length modifier "q", to unbreak > MacOS. > > Optional work: try to turn the run-time failure into a compile- > time failure, ideally in configure. Accepts garbage (although -Wformat detection will help avoid the worst of it). You can't check string equality in the preprocessor, and you can't do things like "case PRId64[0]: case 'l':" to force compilation errors due to duplicate labels, but we CAN limit ourselves to the preprocessor and grep that output to see what PRId64 expands to. I'll see if I can come up with a configure check. Still, it may be easier to just fail configure and tell people to report the bug on their odd libc, at which point we update json-lexer.c and configure, than it is to try and reuse configure results in json-lexer.c (since the PRId64 string is not a constant width, it gets hard to code an exact value into our lexer state machine, but easier to code every reported value). > > b. Support exactly the host's PRId64 and PRIu64 values. > > Work required: replace support of "I64d" and "I64u" by support of > PRId64 and PRIu64. Easy enough in parse_escape(), but not in > json-lexer.c. We could perhaps have the lexer accept the shortest > string that's followed by a conversion specifier as length > modifier, then reject invalid length modifiers in a semantic > action. Interesting idea. I'm playing with it... > > Optional work: try to simplify code that currently works around > unavailability of PRId64 and PRIu64. > > Preferences? > > I like 2b, but I'm not sure I'll like the code implementing it. One way > to find out... In the lexer, widen the state machine to accept up to three unknown characters between % and d. Hmm, there's also the possibility of int64_t being mapped to %jd, in addition to our known culprits of %ld, %lld, %I64d, and %qd. > >> Overall, I like the patch, but we need to fix the problems for the next >> round of this series. > > Yes. At this point, I think I'll spin the next round. But it's not longer a 2.10 priority, so it may be a few days. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
