Bug 221130 - libxo anchor roles don't work as documented
Summary: libxo anchor roles don't work as documented
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Phil Shafer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-31 23:28 UTC by Mark Johnston
Modified: 2017-08-17 18:56 UTC (History)
1 user (show)

See Also:


Attachments
test program (219 bytes, text/x-csrc)
2017-08-01 20:42 UTC, Mark Johnston
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Johnston freebsd_committer freebsd_triage 2017-07-31 23:28:14 UTC
I'm trying to use libxo anchor roles in a piece of code, but they're not working as described in xo_format(5):

     To give a width directly, encode it as the content of the anchor tag:

               xo_emit("({[:10}{:min/%d}/{:max/%d}{]:})\n", min, max);

     To pass a width as an argument, use "%d" as the format, which must appear
     after the "/".  Note that only "%d" is supported for widths.  Using any
     other value could ruin your day.

               xo_emit("({[:/%d}{:min/%d}/{:max/%d}{]:})\n", width, min, max);

However, an attempt to follow this example leads to the width being printed:

$ xo "{[:/%d}{:address/%p}{]:}\n" 18 0xdeadbeef
0x12
$ xo "{[:18}{:address/%p}{]:}\n" 0xdeadbeef
        0xdeadbeef
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2017-07-31 23:30:37 UTC
Phil, could I ask you to take a look at this?
Comment 2 Phil Shafer freebsd_committer freebsd_triage 2017-08-01 05:42:51 UTC
I'll look more tomorrow, but it's a bug in "xo".  The library's working correctly, but "xo" has special magic to treat arguments like varargs, so the bug's likely in there.

Bock [libxo/build]% grep anchor ../tests/core/test_01.c
    xo_emit("anchor {[:/%d}{:address/%p}{]:}\n", 18, NULL);
    xo_emit("anchor {[:18}{:address/%p}{]:}\n", NULL);
Bock [libxo/build]% ./tests/core/test_01.test | grep anchor
anchor                0x0
anchor                0x0
Bock [libxo/build]%

Thanks,
 Phil
Comment 3 Mark Johnston freebsd_committer freebsd_triage 2017-08-01 05:48:13 UTC
Hm, I reported this after hitting a similar problem in some C code, and assumed that the behaviour demonstrated by xo was caused by the same issue. I'll try to come up with a C test case tomorrow; it's possible I just made a mistake somewhere.
Comment 4 Phil Shafer freebsd_committer freebsd_triage 2017-08-01 05:57:29 UTC
Hmmm....  Any chance you had a "{[:/18}" like:

        xo_emit("anchor {[:/18}{:address/%p}{]:}\n", NULL);

I just noticed this does not work, so perhaps that was your earlier issue?

Thanks,
 Phil
Comment 5 Mark Johnston freebsd_committer freebsd_triage 2017-08-01 06:58:31 UTC
Nope, it used a dynamic width following the syntax in the example, but with one argument following the width instead of two.
Comment 6 Phil Shafer freebsd_committer freebsd_triage 2017-08-01 13:04:03 UTC
Not sure I follow you; could you please send the code snippet?

Thanks,
 Phil
Comment 7 Mark Johnston freebsd_committer freebsd_triage 2017-08-01 20:42:18 UTC
Created attachment 184928 [details]
test program

(In reply to Phil Shafer from comment #6)
See the attached test program. When run without any arguments, it does what I expect. But if I add --libxo json, it behaves oddly:

$ ./a.out
        0xdeadc0de
$ ./a.out --libxo json,pretty
{
  "foo": {
    "address": "0x12"$
Comment 8 Phil Shafer freebsd_committer freebsd_triage 2017-08-02 14:31:01 UTC
Comment on attachment 184928 [details]
test program

You need a closing xo_finish() call to allow libxo to close the open contents:

Bock [libxo/build]% ./a.out
        0xdeadc0de
Bock [libxo/build]% ./a.out --libxo json,pretty
{
  "foo": {
    "address": "0xdeadc0de"
  }
}
Bock [libxo/build]% diff -u ~/download/test.c{~,}
--- /Users/phil/download/test.c~	2017-08-02 10:27:54.000000000 -0400
+++ /Users/phil/download/test.c	2017-08-02 10:28:14.000000000 -0400
@@ -9,5 +9,6 @@
 	xo_open_container("foo");
 	xo_emit("{[:/%d}{:address/%p}{]:}\n", 18, 0xdeadc0de);
 	xo_close_container("foo");
+	xo_finish();
 	return (0);
 }
Bock [libxo/build]%

Thanks, 
 Phil
Comment 9 Phil Shafer freebsd_committer freebsd_triage 2017-08-02 17:56:00 UTC
Okay, I've fixed "{[:/18}" and other format strings besides "%d" to work correctly; previously it just gave a warning saying only "%d" was supported.  Also fixed "xo" to properly handle anchor arguments, as well as fixing a bug where I was skipping anchors entirely for non-display output styles, which meant that arguments were not being consumed correctly when "/%d" was used.

This is all in github under the "develop" branch, and will be in the next release that I'll import into HEAD.

But I'd like details on your original problem so ensure I have it fixed.  You said the issue was in C code, so I'm hoping it's either the non-%d fix or the "skipping for non-display styles" one.

Thanks,
 Phil
Comment 10 Mark Johnston freebsd_committer freebsd_triage 2017-08-02 21:46:04 UTC
(In reply to Phil Shafer from comment #8)
Derp. But I still get different behaviour from you. On freefall, I see:

markj@freefall> uname -a
FreeBSD freefall.freebsd.org 12.0-CURRENT FreeBSD 12.0-CURRENT #0 r320066: Wed Jul 12 21:17:55 UTC 2017     peter@build-12.freebsd.org:/usr/obj/usr/src/sys/CLUSTER12  amd64
markj@freefall> ./a.out
        0xdeadc0de
markj@freefall> ./a.out --libxo json,pretty
{
  "foo": {
    "address": "0x12"
  }
}

and, I found that making "address" a string can cause a segfault. That is, if I replace the xo_emit() call with:

    xo_emit("{[:/%d}{:address/%s}{]:}\n", 18, "foo");

I get:

markj@freefall> clang test.c -lxo
markj@freefall> ./a.out 
               foo
markj@freefall> ./a.out --libxo json,pretty
Segmentation fault (core dumped)
markj@freefall>
Comment 11 Phil Shafer freebsd_committer freebsd_triage 2017-08-03 02:50:18 UTC
The "0x12" and the seg fault are both outcomes from the bug "where I was skipping anchors entirely for non-display output styles, which meant that arguments were not being consumed correctly when "/%d" was used".  I'll roll a new release and get it into HEAD.

Thanks,
 Phil
Comment 12 Mark Johnston freebsd_committer freebsd_triage 2017-08-03 03:43:55 UTC
(In reply to Phil Shafer from comment #11)
Perfect, thank you!
Comment 13 Phil Shafer freebsd_committer freebsd_triage 2017-08-03 19:25:10 UTC
Fixes are in HEAD via import of libxo-0.8.4;  I'll MFC this next week info {10,11}-stable.

Thanks,
 Phil
Comment 14 Phil Shafer freebsd_committer freebsd_triage 2017-08-17 18:56:57 UTC
Fixed in HEAD and stable/11.

Thanks,
 Phil