I think we should insert a number of static assertions in the OpenLDAP code: Asserts that fail or succeed at compile time. E.g. for the comment in config.c:bindkey[] (ITS#6419).
Like this - in include/ac/assert.h? Or ldap_cdefs.h in case we want to use it in installed headers?
/* Declaration which tries to force a compile error if !cond */ #define ber_static_assert(cond) \ LBER_V(LDAP_CONST char) ber_assertion[ber_assertz(cond) + 1]
/* Return 0, or if !cond try to force a compile error */ #define ber_assertz(cond) (sizeof(struct { \ int ber_assert1[(cond) ? 9 : -9], ber_assert2:(cond) ? 9 : -9; }) && 0)
/* Usage: */ ber_static_assert(~0U >= 0xFFFFFFFFUL); /* int = 32 bits or wider */ int foo(int i) { return i + ber_assertz(LDAP_SUCCESS == 0); }
liblber will need a dummy variable 'const char ber_assertion[1];'. We can drop that if anyone dislikes it: ber_static_assert() can take an assertion_name parameter and declare a typedef based on that name. Or it can generate a name from __LINE__. Then we can have only one static assert per line, which matters for macros but little else.
Hallvard B Furuseth wrote:
I think we should insert a number of static assertions in the OpenLDAP code: Asserts that fail or succeed at compile time. E.g. for the comment in config.c:bindkey[] (ITS#6419).
Like this - in include/ac/assert.h? Or ldap_cdefs.h in case we want to use it in installed headers?
/* Declaration which tries to force a compile error if !cond */ #define ber_static_assert(cond) \ LBER_V(LDAP_CONST char) ber_assertion[ber_assertz(cond) + 1]
/* Return 0, or if !cond try to force a compile error */ #define ber_assertz(cond) (sizeof(struct { \ int ber_assert1[(cond) ? 9 : -9], ber_assert2:(cond) ? 9 : -9; })&& 0)
/* Usage: */ ber_static_assert(~0U>= 0xFFFFFFFFUL); /* int = 32 bits or wider */ int foo(int i) { return i + ber_assertz(LDAP_SUCCESS == 0); }
liblber will need a dummy variable 'const char ber_assertion[1];'. We can drop that if anyone dislikes it: ber_static_assert() can take an assertion_name parameter and declare a typedef based on that name. Or it can generate a name from __LINE__. Then we can have only one static assert per line, which matters for macros but little else.
You could avoid the naming issue by just enclosing the body in a block {}. I'm assuming we only need this inside functions, and not at file scope.
Since this is all ber/LBER, it should go in an lber_* header file, not ac/assert.h.
Howard Chu writes:
Hallvard B Furuseth wrote:
[Rearranging a little]
Since this is all ber/LBER, it should go in an lber_* header file, not ac/assert.h.
OK. I guess that means lber_pvt.h: ber_pvt_assertz(), ber_pvt_static_assert() since on second thought I don't see much need to export it in installed headers. After all there's nothing BER/LDAP-specific about asserts. Opinions?
liblber will need a dummy variable 'const char ber_assertion[1];'. We can drop that if anyone dislikes it: ber_static_assert() can take an assertion_name parameter and declare a typedef based on that name. Or it can generate a name from __LINE__. Then we can have only one static assert per line, which matters for macros but little else.
Also the __LINE__ variant must not be used in headers, since then we can have name clashes between assertions in different .h/.c files:-(
You could avoid the naming issue by just enclosing the body in a block {}. I'm assuming we only need this inside functions, and not at file scope.
We don't need it, but it's cleaner with the assertion near the code it asserts about - and we do have file-scope assumptions like "<file-scope array X> has length Y", e.g. relay_fail_modes[] in back-relay/op.c.
However ber_pvt_static_assert() is just a convenience wrapper around ber_pvt_assertz(): ber_pvt_static_assert(cond); can be replaced with (void) ber_pvt_assertz(cond); for an {} assert, and enum { some_name = ber_pvt_assertz(cond) }; for a declaration assert.
So whatever we do for ber_pvt_static_assert(), if it doesn't fit the needs of some piece of code it can use one of the latter forms. I prefer it as a declaration since that helps with the ugliest (and longest) syntax.
I too prefer to avoid a name parameter, so if a linker symbol is a problem I suggest the __LINE__ variant for now. We can also add a symbol the macro can use in the future, but not use it yet to keep compatibility with existing binaries.
BTW, yet another trick is to stuff the assertion into the prototype of an existing function: LBER_F(void) ber_pvt_sb_buf_init LDAP_P((Sockbuf_Buf *buf)); becomes LBER_F(void) ber_pvt_sb_buf_init(Sockbuf_Buf buf[assertz expression]); which is supposed to be equivalent - and we already require ISO C for internal files.
I wrote:
I too prefer to avoid a name parameter, so if a linker symbol is a problem I suggest the __LINE__ variant for now. We can also add a symbol the macro can use in the future, but not use it yet to keep compatibility with existing binaries.
Add a linked symbol, I mean - extern const char ber_pvt_assertion[1];
-
Hallvard B Furuseth -
Howard Chu