zfs diff -h/ZFS_DIFF_NO_MANGLE, diff cleanups

[nabijaczleweli]

Motivation and Context

If I have to write another instance of this

#include <stdio.h>
int main() {
	int c;
	while((c = getchar()) != EOF) {
		if(c == '\\') {
			c = 0;
			getchar(); // 0
			c = (c << 3) | (getchar() - '0');
			c = (c << 3) | (getchar() - '0');
			c = (c << 3) | (getchar() - '0');
		}
		putchar(c);
	}
}

to process stuff like this

-       /mnt/filling/store/BKP/D/Dokumenty/Domek/4.\0040sprz\0304\0231ty\0040i\0040materia\0305\0202y/_6\0040pompa\0040ciep\0305\0202a/Ochsner/!\0040panel\0040sterowania\0040w\0040salonie/2018.02\0040DSC_1928\0040-\0040panel\0040-\0040pod\0305\0202\0304\0205czenie\0040kabli.jpg
-       /mnt/filling/store/BKP/D/Dokumenty/b.\0040WCM/3.\0040Przyk\0305\0202ady/1.\0040PlatT/Zarz\0304\0205dzanie\0040Jako\0305\0233ci\0304\0205/tech-karty/IPSTLD\0040-\0040\0320\0242\0320\0265\0321\0205\0320\0275\0320\0276\0320\0273\0320\0276\0320\0263\0320\0270\0321\0207\0320\0265\0321\0201\0320\0272\0320\0260\0321\0217\0040\0320\0272\0320\0260\0321\0200\0321\0202\0320\0260\004028\0320\0245\0320\02232\0320\0242\0320\0240\0320\0243.xls/<xattrdir>

one more time just to read a diff I will return to monky, in a big way

Description

The first patch has the -p/NO_MANGLE flags for easy cherry-picking, the rest are NFC and explained in the messages.

Also, nominally, the %o with uint8_t argument should not work on big endian, but there didn't seem to be any tests validating that?

Went with -p for, dunno, printable? but not so sure about it. thots?

How Has This Been Tested?

Ran it. There's a new test, too, which CI will validate hopefully.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv) – new flag, but no ABI breaks I think?
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied. – CI take my hand
• All commit messages are properly formatted and contain Signed-off-by.

[nabijaczleweli]

uuh FreeBSD HEAD CI failure appears unrelated?

[rincebrain]

Personally, I just use this alias right now, and it's done the inputs I've needed so far, but I can't argue with wanting a builtin to print more human-readably without needing another thing to do it.
alias zfilt="while read -r LINE;do echo -e \"\${LINE}\";done"

[rincebrain]

Seems reasonable to me. Always pleased to see someone sand a rough edge off of zfs diff.

[behlendorf]

Looks good, thanks for the improvement.

[behlendorf]

nit: looks like this cleanup ended up in wrong commit.

[nabijaczleweli]

Fair enough, moved to the ARGSUSED one

[nabijaczleweli]

The Debian 10 and CentOS 7 failures make no sense to me, since they're in casenorm/zpool expansion, which this doesn't touch?

[behlendorf]

It looks like they hit unrelated issues. This should be good to go.

[behlendorf]

Went with -p for, dunno, printable? but not so sure about it.

My only concern with this is we've used -p to mean "parsable" (exact values) in all of the other commands. While that behavior doesn't really apply for zfs diff, it'd probably be wise to leave -p unused (just in case we need it later). What if we used something like -h for human-readable?

[nabijaczleweli]

In many ways this is the exact, unmangled, value, but -h works too, ya – updated

[behlendorf]

Right, I can definitely see the argument for -p. I'm good with -h though, just to be on the safe side. @rincebrain thoughts?

[rincebrain]

-h sounds good to me - conceptually, I'd imagine that the default output of diff is only maybe one step removed from what we'd want from a --parsable anyway - since it's not really intended for direct human consumption, but ease of processing, all I'd really want on top would be some sort of serialization of the state of the from/to objects whenever it can't print the normal representations (e.g. any time it'd print an error).