-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup VERIFY() macros #17163
base: master
Are you sure you want to change the base?
Cleanup VERIFY() macros #17163
Conversation
It looks like these are actually subtly different. ASSERT3B is only casting to
Before this change though, I don't know what I would do, to be honest. The smallest thing, I suppose, is to make (Longer term, I might drop |
Hmm. |
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:
That passes, because both sides are "true" and so "equal", but
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:
Of course, that's just another way of spelling 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 |
@robn How about this version? |
- 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. Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc.
VERIFY3B()
when given non-boolean values.EQUIV()
intoVERIFY3B(,==,)
as equivalent.%px
outside of Linux kernel.Types of changes
Checklist:
Signed-off-by
.