Page MenuHomeFreeBSD

Document two new conventions:
Needs RevisionPublic

Authored by imp on Jan 9 2020, 10:19 PM.



C99 initializers have been encouraged for a while.
_Static_assert is to be preferred to CTASSERT.

Diff Detail

Lint OK
No Unit Test Coverage
Build Status
Buildable 28583
Build 26625: arc lint + arc unit

Event Timeline

imp created this revision.Jan 9 2020, 10:19 PM
julian requested changes to this revision.Jan 9 2020, 10:21 PM
julian added a subscriber: julian.
julian added inline comments.

.. e.g. (example)..
so people don't have to go look it up to see what it is.

This revision now requires changes to proceed.Jan 9 2020, 10:21 PM
brooks added a subscriber: brooks.Jan 9 2020, 10:57 PM

I'd rather see use switch to static_assert since that's in C11 and C++11. It's a bit unfortunate that only C++17 has the single argument version though.

rlibby added a subscriber: rlibby.Jan 9 2020, 11:07 PM
rlibby added inline comments.

Just my opinion: _Static_assert() is longer, uglier, and the mandatory second argument is often not useful (since it's just a string, and not e.g. a printf format). It feels weird to have regular code directly invoking reserved identifiers (underscore capital), like directly using _Bool. I'd suggest we consider just leaving CTASSERT() alone and maybe adding a new CTASSERT1() wrapper.

melifaro added inline comments.Jan 9 2020, 11:17 PM

I'd second this: _Static_assert() looks weird as it feels like reserved macro. Additionally, having a macro able to provide an error explanation directly without encoding it in the comments is pretty useful. I'd prefer either sticking with static_assert() or introducing CTASSERT1() wrapper.

jhb added a comment.Jan 10 2020, 1:25 AM

From here: it seems that the intention is that _Static_assert should be used publicly as static_assert() from <assert.h>. (Our assert.h defines it). This is indeed similar to _Bool vs bool where a header is used in userland (<stdbool.h>) to give the C++ name in C. What we have one for 'bool' in the kernel is to go ahead and define 'bool' in <sys/types.h> under #ifdef _KERNEL. I think something similar is probably ok in in <sys/systm.h> where we define 'static_assert' to '_Static_assert' and fix existing kernel code to use the C++ spelling.

I agree with @jhb, we should prefer static_assert using assert.h