Bug 200746 - [PATCH] Add libxo support to jls
Summary: [PATCH] Add libxo support to jls
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Craig Rodrigues
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-06-09 21:27 UTC by Emmanuel Vadot
Modified: 2015-12-22 01:34 UTC (History)
4 users (show)

See Also:


Attachments
jls libxo support (8.88 KB, patch)
2015-06-09 21:27 UTC, Emmanuel Vadot
no flags Details | Diff
jls libxo support (8.96 KB, patch)
2015-06-10 05:36 UTC, Emmanuel Vadot
no flags Details | Diff
jls libxo support 3 (10.77 KB, patch)
2015-06-13 20:26 UTC, Emmanuel Vadot
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emmanuel Vadot 2015-06-09 21:27:38 UTC
Created attachment 157588 [details]
jls libxo support

Hello,

This patch add libxo support to jls.
It also correct an error in -q switch (the value was always quoted).

For boolean values, text is the same, for json and xml it is set to true/false.
For multiple value (ip for example), for json it's on a array.

Here is some sample outputs :

$ jls -qnh --libxo json | python2.7 -m json.tool


{
    "jails": [
        {
            "allow.chflags": false,
            "allow.mount": false,
            "allow.mount.devfs": true,
            "allow.mount.fdescfs": false,
            "allow.mount.nullfs": false,
            "allow.mount.procfs": false,
            "allow.mount.tmpfs": false,
            "allow.mount.zfs": false,
            "allow.quotas": false,
            "allow.raw_sockets": false,
            "allow.set_hostname": true,
            "allow.socket_af": false,
            "allow.sysvipc": false,
            "children.cur": "0",
            "children.max": "0",
            "cpuset.id": "2",
            "devfs_ruleset": "4",
            "dying": false,
            "enforce_statfs": "2",
            "host": "new",
            "host.domainname": "\"\"",
            "host.hostid": "0",
            "host.hostname": "jail1",
            "host.hostuuid": "6568ce83-0d95-11e5-a67f-44a8420e7030",
            "ip4": "disable",
            "ip4.addr": "10.0.0.2",
            "ip4.saddrsel": true,
            "ip6": "disable",
            "ip6.saddrsel": true,
            "jid": "5",
            "name": "ioc-6568ce83-0d95-11e5-a67f-44a8420e7030",
            "osreldate": "1100076",
            "osrelease": "11.0-CURRENT",
            "parent": "0",
            "path": "/iocage/jails/6568ce83-0d95-11e5-a67f-44a8420e7030/root",
            "persist": true,
            "securelevel": "2"
        }
    ]
}

$ jls -qnh --libxo xml                                                                                                                                                                              
<jails><devfs_ruleset>4</devfs_ruleset><dying>false</dying><enforce_statfs>2</enforce_statfs><host>new</host><ip4>disable</ip4><ip6>disable</ip6><jid>5</jid><name>ioc-6568ce83-0d95-11e5-a67f-44a8420e7030</name><osreldate>1100076</osreldate><osrelease>11.0-CURRENT</osrelease><parent>0</parent><path>/iocage/jails/6568ce83-0d95-11e5-a67f-44a8420e7030/root</path><persist>true</persist><securelevel>2</securelevel><allow.chflags>false</allow.chflags><allow.mount>false</allow.mount><allow.mount.devfs>true</allow.mount.devfs><allow.mount.fdescfs>false</allow.mount.fdescfs><allow.mount.nullfs>false</allow.mount.nullfs><allow.mount.procfs>false</allow.mount.procfs><allow.mount.tmpfs>false</allow.mount.tmpfs><allow.mount.zfs>false</allow.mount.zfs><allow.quotas>false</allow.quotas><allow.raw_sockets>false</allow.raw_sockets><allow.set_hostname>true</allow.set_hostname><allow.socket_af>false</allow.socket_af><allow.sysvipc>false</allow.sysvipc><children.cur>0</children.cur><children.max>0</children.max><cpuset.id>2</cpuset.id><host.domainname>""</host.domainname><host.hostid>0</host.hostid><host.hostname>jail1</host.hostname><host.hostuuid>6568ce83-0d95-11e5-a67f-44a8420e7030</host.hostuuid><ip4.addr>10.0.0.2/ip4.addr><ip4.saddrsel>true</ip4.saddrsel><ip6.saddrsel>true</ip6.saddrsel></jails>

The diff is attached and also on the libxo_support branch of my fork on github (https://github.com/evadot/freebsd/tree/libxo_support)

Cheers.
Comment 1 Emmanuel Vadot 2015-06-10 05:36:32 UTC
Created attachment 157601 [details]
jls libxo support

Update patch to correctly handle empty values.
Comment 2 Emmanuel Vadot 2015-06-13 20:26:12 UTC
Created attachment 157719 [details]
jls libxo support 3

Explicitly use padding for spaces and newline
Fix a bug when WITHOUT_INET
Replace multiple spaces by /%XXs
Comment 3 Jamie Gritton freebsd_committer freebsd_triage 2015-06-23 17:07:50 UTC
A few things I've noticed when looking through the diff.  Bear in mind that I've only the slightest passing familiarity with what libxo does :-).

What's the standard in man pages for adding libxo support (or what will be)?  I see the --libxo option mentioned in the synopsis, but that's it.  I suppose there should also be a reference to some xo page though I don't know which.  And maybe it's just pride, but I'd prefer the --libxo option be moved to after the jls-specific options (or perhaps just before [-j jail]).

One of the xo_emit calls kind of jumped out at me, on line 225:
xo_emit(params[i].jp_name, stdout);
I'd expect something more like
xo_emit("%s", params[i].jp_name);
There are other cases where (slightly) untrusted values are passed in the format string, mostly parameter names which are highly unlikely to have formatting characters, but also the value in quote_print().

In print_jail(), I see an inconsistency in the duplicated arguments.  In particular, there's a "-" in lines 442 and 43, but a "" in line 434 to match the "-" in line 436.  I assume you don't want the dashes in the libxo output, so probably 442 should have "".

There are a few issues I have with quoted_print()...

I see you've altered it to pass in pflags and only actually print the quote characters when they were in fact wanted.  That's definitely the right thing to do, and I really don't know how it went so long without being noticed.  But it should stand on its own, probably as a separate bug report (at least as a separate commit).  And I'd do it a bit differently, checking the flag where quoted_print is called rather than passing that to inside the function - or perhaps going the other way and putting all of the surrounding code block (lines 503-510) into the function, and calling it something more like print_value().

On the minus side, you've removed some necessary code from quoted_print().  It used to backslash-escape quote characters inside a quoted string.  Granted, that's something of a corner case, given that it would only happen when a value contains both double and single quote marks.  But such a value could exist, and should still be handled.  I notice none of this really matters to the libxo output, since it has its own quoting/escaping conventions, but the traditional output still needs it.

Also, any particular reason you removed the comments from quoted_print()?  I suppose they're pretty self-evident, but sometimes even the self-evident ones can be handy.  In fact, I should have also included the backslash-escape bit in the comment.
Comment 4 Jason Unovitch freebsd_committer freebsd_triage 2015-07-21 01:08:59 UTC
(In reply to Emmanuel Vadot from comment #0)
A small tidbit:  '--libxo json,pretty' should produce the same output you are piping your program to python to handle.
Comment 5 commit-hook freebsd_committer freebsd_triage 2015-12-22 00:59:37 UTC
A commit references this bug:

Author: rodrigc
Date: Tue Dec 22 00:58:36 UTC 2015
New revision: 292580
URL: https://svnweb.freebsd.org/changeset/base/292580

Log:
  Add libxo support to jls

  PR:                    200746
  Submitted by:          Emmanuel Vadot <manu bidouilliste com>
  Reviewed by:           allanjude
  Relnotes:              yes
  Differential Revision: https://reviews.freebsd.org/D4452

Changes:
  head/usr.sbin/jls/Makefile
  head/usr.sbin/jls/jls.8
  head/usr.sbin/jls/jls.c
Comment 6 Craig Rodrigues freebsd_committer freebsd_triage 2015-12-22 01:34:15 UTC
Thanks for submitting this, Emmanuel!!