mount(8): Add libxo(3) support

[me_cameronkatri.com]

Test Plan

• Text output is the same.

[me_cameronkatri.com]

Use no quote modifier on dump and pass.

[me_cameronkatri.com]

Add full context.

[otis]

sbin/mount/mount.c

151 ↗

(On Diff #89485)

Aren't these lines (151, 152) a suitable candidate for a macro?

[me_cameronkatri.com]

Open the mount container before any calls to xo_errx
Use a macro to close the container, finish and exit.

[imp]

I think the xo_emit() stuff is right, but I'm not sure.
The rest of the stuff seems good, modulo my comments.

sbin/mount/mount.c

69 ↗

(On Diff #89553)

Minor nit: I'd align all the \ if they aren't already (it's hard to tell with phab sometimes, but I think there's a tab missing)

257 ↗

(On Diff #89553)

This doesn't make any sense. Negative return values from main are weird.
Better to do a xo_errx(1, "Can't parse arges") (or w/o the xo_ prefix if this isn't allowed). Return '1' if there's already an error message printed so it isn't doubled.

675 ↗

(On Diff #89553)

this is indented too far. It should just be tab 4-spaces.

[me_cameronkatri.com]

Check argc after xo_parse_args().

[me_cameronkatri.com]

Revert last diff (uploaded to wrong differential)

[me_cameronkatri.com]

Fix some styling.

[me_cameronkatri.com]

sbin/mount/mount.c

69 ↗

(On Diff #89553)

This looks aligned in vim with freebsd.vim.

[pstef]

sbin/mount/mount.c

711 ↗

(On Diff #94127)

I'm not sure I understand the purpose of this, as asprintf will allocate the buffer and set fsidbuf to point to it.

713 ↗

(On Diff #94127)

Probably should check the return value of asprintf.

911 ↗

(On Diff #94127)

Should we update usage()?

[me_cameronkatri.com]

Check the return value of asprintf.

[me_cameronkatri.com]

sbin/mount/mount.c

711 ↗

(On Diff #94127)

If I don't do this, when using --libxo json the fsid will start with (null), I'm sure there is a better way to stop that but this worked.

911 ↗

(On Diff #94127)

Should we update usage()?

It doesn't look like any other commands show --libxo in their usage.

[pstef]

sbin/mount/mount.c

711 ↗

(On Diff #94127)

There are some 32-bit ints in fsid_t and we want to interpret them as unsigned bytes and print using hexadecimal representation. So it's a matter of calculating the needed space once and using that space in a loop, like this:

		if (sfp->f_fsid.val[0] != 0 || sfp->f_fsid.val[1] != 0) {
			xo_emit("{D:, }{Lw:fsid}");
			fsidbuf = malloc(sizeof(sfp->f_fsid) * 2 + 1);
			if (fsidbuf == NULL)
				xo_errx(1, "malloc failed");
			for (i = 0; i < sizeof(sfp->f_fsid); i++)
				sprintf(&fsidbuf[i * 2], "%02x",
				    ((u_char *)&sfp->f_fsid)[i]);
			fsidbuf[i * 2] = '\0';
			xo_emit("{:fsid}", fsidbuf);
			free(fsidbuf);
		}

[pstef]

sbin/mount/mount.c

711 ↗

(On Diff #94127)

And then it seems the two xo_emit() calls can be folded into xo_emit("{D:, }{Lw:fsid}{:fsid}", fsidbuf);

[me_cameronkatri.com]

Change fsid allocation. Thanks pstef.

[phil]

xo_parse_args gives error messages for invalid arguments; no further error is needed.

https://libxo.readthedocs.io/en/latest/api.html?#parsing-command-line-arguments-xo-parse-args

argc = xo_parse_args(argc, argv);
if (argc < 0)
    exit(EXIT_FAILURE);

Also looks like you've lost the space in "write: sync". Use "{Lwc:writes}{P: }{Lw:sync}".

Could you please give me more details about your comment "If I don't do this, when using --libxo json the fsid will start with (null),", since we don't want that to be happening?

Thanks,
Phil

[phil]

One more thing: Does libxo need an xo_exit() function? It would be just xo_finish + exit, like your EXIT macro, but might help minimize diffs like this.

Thanks,
Phil

[me_cameronkatri.com]

In D30341#727179, @phil wrote:

Also looks like you've lost the space in "write: sync". Use "{Lwc:writes}{P: }{Lw:sync}".

diff -u <(sudo /usr/obj/usr/src/amd64.amd64/sbin/mount/mount -v) <(sudo mount -v) doesn't show any difference between the outputs before and after this commit.

[pstef]

In D30341#727179, @phil wrote:

Could you please give me more details about your comment "If I don't do this, when using --libxo json the fsid will start with (null),", since we don't want that to be happening?

That was about a previous version of this diff and it had more to do with memory management and strings than with libxo.