Bug 244389 - [libxo] procstat(1) --libxo=xml produces invalid markup
Summary: [libxo] procstat(1) --libxo=xml produces invalid markup
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.0-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-25 02:12 UTC by Thomas Hurst
Modified: 2020-05-27 14:58 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Hurst 2020-02-25 02:12:20 UTC
-% procstat binary $$ --libxo=xml,pretty
<procstat version="1">
  <binary>
    <48700>
      <process_id>48700</process_id>
      <command>zsh</command>
      <osrel>1200086</osrel>
      <pathname>/usr/local/bin/rzsh</pathname>
    </48700>
  </binary>
</procstat>

Numeric tag names are not valid in XML:

malformed XML: missing tag start
Line: 10 Position: 225
Last 80 unconsumed characters:
<48700>       <process_id>48700</process_id>       <command>zsh</command>

Looks like this was reported in bug #213478 and closed as fixed, but it's clearly not.

I'm not sure what libxo should do about this, perhaps just prefix the tag name with an underscore, but arguably procstat(1) should follow ps(1) and make a list of processes rather than opening a container for each one.
Comment 1 Phil Shafer freebsd_committer 2020-05-27 14:58:59 UTC
Yes, this is completely broken in a conceptual way.  The tags in libxo should be thought of as "columns" in a database, not as data.  Using a pid as the tag here is wrong; it should be more like:

...
 <binary>
   <process>
      <pid>48700</pid>
      <command>zsh</command>
      ...

The big question is: how do we manage changes to libxo-based encoding?  If we make this change (or other future changes), then there is a risk of breaking something that depends on it?  How do we announce changes that are not backwards compatible?  Do we just declare is a "bug" that we fix and announce via release notes?  Do we need to start using xo_version() calls?  Do we enforce the use of xo_version at the first change?  How do we arrive on a policy that we are all comfortable with?

In this case, it's a obvious bug in XML, but folks using JSON might be depending on it, so it's a semi-perfect example.

Thanks,
 Phil