Cleanup VERIFY() macros

[amotin]

• Fix VERIFY3B() when given non-boolean values.
• Map EQUIV() into VERIFY3B(,==,) as equivalent.
• Tune messages for better readability and to closer match source code for easier search. Unify user-space messages with kernel.
• Tune printed types and remove %px outside of Linux kernel.

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)
• 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.
• All commit messages are properly formatted and contain Signed-off-by.

[robn]

It looks like these are actually subtly different.

ASSERT3B is only casting to boolean_t, but that's an enum, so really an integer. So ASSERT3B(5, 3) actually trips:

ASSERT at cmd/zdb/zdb.c:9259:main()
5 == 3 (0x5 == 0x3)

Before this change though, EQUIV(5, 3) uses !! to convert the values to "truthiness", so EQUIV(5, 3) doesn't trip. With this change, it's just ASSERT3B, and trips.

I don't know what I would do, to be honest. The smallest thing, I suppose, is to make ASSERT3B use !! as well, and it looks like it will be safe with existing uses. But I won't insist for the moment; if you want to proceed with this as-is, I'll approve it.

(Longer term, I might drop ASSERT3B entirely, and have more "named" asserts for booleans, like we already have with EQUIV and IMPLY. I've not thought about this much at all though.)

[amotin]

Hmm. boolean_t is indeed an enum. But then ASSERT3B() is just broken already unless we add there explicit !! or != 0. @robn, any reason why you want it dropped rather than fixed?

[robn]

Oh for now I would just fix it by adding !!.

My rough thinking behind dropping it is that the normal operators don't quite "feel" right for booleans. Eg, this example:

ASSERT3B(5, ==, 3)

That passes, because both sides are "true" and so "equal", but == looks so weird to my eyes there. So maybe it should take more "logic" operators?

ASSERT3B(5, &&, 3)

It's sorta more what it means, but again, it looks weird to me. So that's when I start wondering having the operator spelled out in the macro name makes more sense:

ASSERT_AND(5, 3)

Of course, that's just another way of spelling EQUIV(...), so we already have an idea that the named ones are nicer. So maybe that is the way forward?

But as I said, I've barely thought about this. Before I made a proper suggestion I'd go and see what other codebases have done, and to what extent we use boolean asserts, and what other asserts we use where we could have used a boolean assert instead.

So unless you're really in the mood, for now I reckon just fix ASSERT3B and move on :)

[amotin]

@robn How about this version?

[robn]

Looks good. Sorry for the delay!