Eric Blake <ebl...@redhat.com> writes: > Commit 1792d7d0 was written because PRId64 expands to non-portable > crap for some libc, and we had testsuite failures on Mac OS as a > result. This in turn makes it difficult to rely on the obvious > conversions of 64-bit values into JSON, requiring things such as > casting int64_t to long long so we can use a reliable %lld and > still keep -Wformat happy. So now it's time to fix that. > > We are already lexing %I64d (hello mingw); now expand the lexer > to parse %qd (hello Mac OS). In the process, we can drop one > state: IN_ESCAPE_I64 is redundant with IN_ESCAPE_LL, and reused > again by %qd, so rename it to IN_ESCAPE_64. And fix a comment > that mistakenly omitted %u as a supported escape. > > Next, tweak the parser to accept the exact spelling of PRId64 > regardless of what it expands to (note that there are are now dead > branches in the 'if' chain for some platforms, like glibc; but > there are other platforms where all branches are necessary). We > are at least safe in that we are parsing the correct size 32-bit or > a 64-bit quantity on whatever branch we end up in. This also means > that we no longer parse everything that the lexer will consume (no > more %I64d on glibc), but that is intentional. And of course, > update the testsuite for coverage of the feature. > > Finally, update configure to barf on any libc that uses yet > another form of PRId64 that we have not yet coded for, so that we > can once again update json-lexer.c to cater to those quirks (my > guess? we might see %jd next) (on the bright side, json-parser.c > should no longer need updates). Yes, the only way I could find > to quickly learn what spelling is preferred when cross-compiling > is to grep a compiled binary; C does not make it easy to turn a > string constant into an integer constant, let alone make > preprocessor decisions; and even parsing '$CC -E' output is > fragile, since 64-bit glibc pre-processes as "l" "d" rather than > "ld", and newer gcc splits macro expansions across multiple lines. > I'm assuming that 'strings' is portable enough during configure; > if that assumption fails in some constrained docker environment, > an easy hack is to add this to configure: > strings () { tr -d '\0' < "$1"; } > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v3: incorporate review comments from Markus > --- > configure | 24 ++++++++++++++++++++++++ > qobject/json-lexer.c | 21 +++++++++------------ > qobject/json-parser.c | 10 ++++++---- > tests/check-qjson.c | 7 +++++++ > 4 files changed, 46 insertions(+), 16 deletions(-) > > diff --git a/configure b/configure > index 987f59ba88..5b0eddec3c 100755 > --- a/configure > +++ b/configure > @@ -3222,6 +3222,30 @@ for i in $glib_modules; do > fi > done > > +# Sanity check that we can parse PRId64 and friends in json-lexer.c > +# (Sadly, the "easiest" way to do this is to grep the compiled binary, > +# since we can't control whether the preprocessor would output "ld" vs. > +# "l" "d", nor even portably ensure it fits output on the same line as > +# a witness marker.) > +cat > $TMPC <<EOF > +#include <inttypes.h> > +int main(void) { > + static const char findme[] = "\nUnLiKeLyToClAsH_" PRId64 "\n"; > + return !findme[0]; > +} > +EOF > +if ! compile_prog "$CFLAGS" "$LIBS" ; then
Would compile_object suffice? > + error_exit "can't determine value of PRId64" > +fi > +nl=' > +' > +case $(strings $TMPE | grep ^UnLiKeLyToClAsH) in > + '' | *"$nl"* ) error_exit "can't determine value of PRId64" ;; > + *_ld | *_lld | *_I64d | *_qd) ;; > + *) error_exit "unexepected value of PRId64, please add %$(strings $TMPE | > + sed -n s/^UnLiKeLyToClAsH_//p) support to json-lexer.c" ;; > +esac > + > # Sanity check that the current size_t matches the > # size that glib thinks it should be. This catches > # problems on multi-arch where people try to build > diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c > index 980ba159d6..9a0fc58444 100644 > --- a/qobject/json-lexer.c > +++ b/qobject/json-lexer.c > @@ -29,9 +29,11 @@ > * > * '([^\\']|\\[\"'\\/bfnrt]|\\u[0-9a-fA-F]{4})*' > * > - * Extension for vararg handling in JSON construction: > + * Extension for vararg handling in JSON construction [this lexer > + * actually accepts multiple forms of PRId64, but parse_escape() later > + * filters to only parse the current platform's spelling]: I'd stick to (parenthesis) instead of [square brackets] here. > * > - * %((l|ll|I64)?d|[ipsf]) > + * %(PRI[du]64|(l|ll)?[ud]|[ipsf]) > * > */ > [...] Reviewed-by: Markus Armbruster <arm...@redhat.com>