sys: Stop requiring nested parentheses for KASSERT

[jhb]

Summary

Permit passing the panic format string and arguments as additional
arguments to KASSERT itself. This is easier to read and can be less
confusing for style checkers, etc.

Note that to preserve API compatability for now, if only two arguments
are passed to the KASSERT, the second argument must still use nested
parantheses. Eventually we should remove this compatability.

While here, take advantage of the wrapper macro that provides API
compatability to add a single argument form (KASSERT(expression))
which auto-generates a message identical to MPASS. This does mean
that the wrapper macro (_KASSERT_MACRO) will need to stay even after
removing the API compatability for the nested parantheses in the
future.

[jrtc27]

Doesn't the use of _KASSERT_MACRO artificially limit the number of printf arguments, including format string, to 11 (possibly off by one)? Whilst parenthesising in that case is fine for now, I don't think we want to have that limitation long-term, especially once the parenthesised form is gone...

[kib]

share/man/man9/KASSERT.9

172

I suggest to leave the MPASS() example as well. It is useful documentation to understand the existing code.

[jhb]

In D45981#1048417, @jrtc27 wrote:

Doesn't the use of _KASSERT_MACRO artificially limit the number of printf arguments, including format string, to 11 (possibly off by one)? Whilst parenthesising in that case is fine for now, I don't think we want to have that limitation long-term, especially once the parenthesised form is gone...

It does, though if we need more arguments in the future we can expand the macro to work. I just chose 10 as a SWAG for the max arguments for now. If we didn't care about the single argument version we could eventually drop _KASSERT_MACRO entirely, but if we want the single argument version to work we are stuck with it I think.

[jrtc27]

In D45981#1048465, @jhb wrote:

In D45981#1048417, @jrtc27 wrote:

Doesn't the use of _KASSERT_MACRO artificially limit the number of printf arguments, including format string, to 11 (possibly off by one)? Whilst parenthesising in that case is fine for now, I don't think we want to have that limitation long-term, especially once the parenthesised form is gone...

It does, though if we need more arguments in the future we can expand the macro to work. I just chose 10 as a SWAG for the max arguments for now. If we didn't care about the single argument version we could eventually drop _KASSERT_MACRO entirely, but if we want the single argument version to work we are stuck with it I think.

Given that not giving a message is lazy and bad practice that should not be encouraged, I'm loathe to add such hacks and restrictions just to support it. There's always MPASS if you really want it, or we can add a KASSERT1, but it's worth pointing out that MPASS at least gives you the file and line number when there's no message, unlike KASSERT which will just print the condition with no context other than what DDB can infer.

[jhb]

In D45981#1048466, @jrtc27 wrote:

In D45981#1048465, @jhb wrote:

In D45981#1048417, @jrtc27 wrote:

Doesn't the use of _KASSERT_MACRO artificially limit the number of printf arguments, including format string, to 11 (possibly off by one)? Whilst parenthesising in that case is fine for now, I don't think we want to have that limitation long-term, especially once the parenthesised form is gone...

It does, though if we need more arguments in the future we can expand the macro to work. I just chose 10 as a SWAG for the max arguments for now. If we didn't care about the single argument version we could eventually drop _KASSERT_MACRO entirely, but if we want the single argument version to work we are stuck with it I think.

Given that not giving a message is lazy and bad practice that should not be encouraged, I'm loathe to add such hacks and restrictions just to support it. There's always MPASS if you really want it, or we can add a KASSERT1, but it's worth pointing out that MPASS at least gives you the file and line number when there's no message, unlike KASSERT which will just print the condition with no context other than what DDB can infer.

Err, I made KASSERT() match MPASS() in its output when used with a single argument.

[jrtc27]

In D45981#1048615, @jhb wrote:

In D45981#1048466, @jrtc27 wrote:

In D45981#1048465, @jhb wrote:

In D45981#1048417, @jrtc27 wrote:

Doesn't the use of _KASSERT_MACRO artificially limit the number of printf arguments, including format string, to 11 (possibly off by one)? Whilst parenthesising in that case is fine for now, I don't think we want to have that limitation long-term, especially once the parenthesised form is gone...

It does, though if we need more arguments in the future we can expand the macro to work. I just chose 10 as a SWAG for the max arguments for now. If we didn't care about the single argument version we could eventually drop _KASSERT_MACRO entirely, but if we want the single argument version to work we are stuck with it I think.

Given that not giving a message is lazy and bad practice that should not be encouraged, I'm loathe to add such hacks and restrictions just to support it. There's always MPASS if you really want it, or we can add a KASSERT1, but it's worth pointing out that MPASS at least gives you the file and line number when there's no message, unlike KASSERT which will just print the condition with no context other than what DDB can infer.

Err, I made KASSERT() match MPASS() in its output when used with a single argument.

Ah, right. But then why do we need two macros that do exactly the same thing?

[imp]

I'd ditch the single arg version and any hacks to support it. Especially since it's limiting us, bad practice and adds a new variant.
Other than the limitations, I like this.

[jhb]

I'd kind of wanted to kill MPASS eventually since I have used it for all sorts of assertions that have nothing to do with SMP, and other assertions in the language permit 1 argument variants (assert() and static_assert()). However, it is simpler to not have the single arg version and it does mean _KASSERT_MACRO would be temporary. We could always add a single arg variant later if we wanted.

[emaste]

In D45981#1048758, @jhb wrote:

I'd kind of wanted to kill MPASS eventually since I have used it for all sorts of assertions that have nothing to do with SMP, and other assertions in the language permit 1 argument variants (assert() and static_assert()).

I believe it was @mhorne who bacronymed MPASS to Must Pass ASSert and that's one way to address this :)

One of the benefits of a one-arg KASSERT is that it is obvious to anyone already familiar with C what it implies, and personally I like offering one-arg KASSERT. But if others disagree and the consensus is to do only the >= 2 arg version initially that's fine with me.

[mhorne]

In D45981#1048758, @jhb wrote:

I'd kind of wanted to kill MPASS eventually since I have used it for all sorts of assertions that have nothing to do with SMP, and other assertions in the language permit 1 argument variants (assert() and static_assert()). However, it is simpler to not have the single arg version and it does mean _KASSERT_MACRO would be temporary. We could always add a single arg variant later if we wanted.

Are you saying it would become easier to support the single arg version after fully transitioning from the old-style KASSERT (and dropping the API compat)? In that case a two-staged transition makes sense, although it would have to happen over a longer time-frame.

I agree it would be ideal to replace MPASS with a single-arg KASSERT, if it can be cleanly done. If not we still should find-and-replace MPASS to be something like KASSERT0, or KASSERT1, and it would be an improvement.

[jhb]

In D45981#1048761, @mhorne wrote:

In D45981#1048758, @jhb wrote:

I'd kind of wanted to kill MPASS eventually since I have used it for all sorts of assertions that have nothing to do with SMP, and other assertions in the language permit 1 argument variants (assert() and static_assert()). However, it is simpler to not have the single arg version and it does mean _KASSERT_MACRO would be temporary. We could always add a single arg variant later if we wanted.

Are you saying it would become easier to support the single arg version after fully transitioning from the old-style KASSERT (and dropping the API compat)? In that case a two-staged transition makes sense, although it would have to happen over a longer time-frame.

I agree it would be ideal to replace MPASS with a single-arg KASSERT, if it can be cleanly done. If not we still should find-and-replace MPASS to be something like KASSERT0, or KASSERT1, and it would be an improvement.

The current patch supports a single arg version now. Having a single arg version requires having the _KASSERT_MACRO helper macro forever whereas if we don't do that then KASSERT can eventually just be the KASSERTN macro from the patch after the transition period is over. It's really two separate changes, but both changes make use of the _KASSERT_MACRO approach so somewhat convenient to do both at once if we want the single-arg version.

[bz]

sys/sys/kassert.h

155

Silly question: can we have the same message from KASSERT1 here too and concat it with the "msg" given; that would allow us to write a lot simpler messages in the future AND make sure func/line or file/func/line are always given.

I do understand for a while we may have long messages but I am almost willing to make a pass over the tree to edit most of them down once this is in to clear things up.

160

func is often a lot more helpful than file -- at least one knows right from the message where in that file it happened (and has context); can we add func?

[erj]

sys/sys/kassert.h

160

I approve of that suggestion!