Hi On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering <lenn...@poettering.net> wrote: > On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote: > >> 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support >> for looking up names) broke the build on clang. >> >> src/resolve/resolved-manager.c:553:43: error: non-const static data >> member must be initialized out of line >> uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct >> in6_pktinfo))) >> >> Moving the MAX(...) to a separate line fixes that problem but another >> error then happens: >> >> src/resolve/resolved-manager.c:554:25: error: fields must have a >> constant size: 'variable length array in structure' extension will >> never be supported >> uint8_t buffer[CMSG_SPACE(size) >> >> We have encountered the same problem before and Lennart was able to >> write the code in a different way. Would this be possible here too? > > My sugegstion here would be to maybe rewrite the MAX() macro to use > __builtin_constant_p() so that it becomes constant if the params are > constant, and only uses code block when it isn't. Or so... > > http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html
Hm, I don't know whether that works. See the description here: https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html What you propose is something like my attached patch, I guess? Along the lines of: (__builtin_constant_p(A) && __builtin_constant_p(B)) ? ((A) > (B) ? (A) : (B)) : ({ ....OLD_MAX.... }) Thing is, the ELSE case is not considered a compile-time constant by LLVM. Therefore, the whole expression is not considered a compile-time constant. I don't know whether conditions with __builtin_constant_p() are evaluated at the parser-step. The GCC example replaces the ELSE case with -1, effectively making both compile-time constants. I also added a test-case to make sure __builtin_constant_p doesn't evaluate the arguments. Can someone with LLVM set up give this a spin? Thanks David diff --git a/src/shared/macro.h b/src/shared/macro.h index 5619c32..18f5a79 100644 --- a/src/shared/macro.h +++ b/src/shared/macro.h @@ -140,6 +140,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) { _a > _b ? _a : _b; \ }) +#undef MAX +#define MAX(_A, _B) \ + __extension__ ( \ + (__builtin_constant_p(_A) && __builtin_constant_p(_B)) \ + ? (((_A) > (_B)) ? (_A) : (_B)) \ + : ({ \ + typeof(_A) _a = (_A); \ + typeof(_B) _b = (_B); \ + _a > _b ? _a : _b; \ + })) + #define MAX3(x,y,z) \ __extension__ ({ \ typeof(x) _c = MAX(x,y); \ diff --git a/src/test/test-util.c b/src/test/test-util.c index 1850f97..d348ac5 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -70,6 +70,20 @@ static void test_align_power2(void) { } } +static void test_max(void) { + /* try triggering compile-errors for non-const initializers */ + static const struct { + int a; + } val1 = { + .a = MAX(10, 100), + }; + int d = 0; + + assert_se(val1.a == 100); + assert_se(MAX(++d, 0) == 1); + assert_se(d == 1); +} + static void test_first_word(void) { assert_se(first_word("Hello", "")); assert_se(first_word("Hello", "Hello")); @@ -926,6 +940,7 @@ int main(int argc, char *argv[]) { test_streq_ptr(); test_align_power2(); + test_max(); test_first_word(); test_close_many(); test_parse_boolean();
diff --git a/src/shared/macro.h b/src/shared/macro.h index 5619c32..18f5a79 100644 --- a/src/shared/macro.h +++ b/src/shared/macro.h @@ -140,6 +140,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) { _a > _b ? _a : _b; \ }) +#undef MAX +#define MAX(_A, _B) \ + __extension__ ( \ + (__builtin_constant_p(_A) && __builtin_constant_p(_B)) \ + ? (((_A) > (_B)) ? (_A) : (_B)) \ + : ({ \ + typeof(_A) _a = (_A); \ + typeof(_B) _b = (_B); \ + _a > _b ? _a : _b; \ + })) + #define MAX3(x,y,z) \ __extension__ ({ \ typeof(x) _c = MAX(x,y); \ diff --git a/src/test/test-util.c b/src/test/test-util.c index 1850f97..d348ac5 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -70,6 +70,20 @@ static void test_align_power2(void) { } } +static void test_max(void) { + /* try triggering compile-errors for non-const initializers */ + static const struct { + int a; + } val1 = { + .a = MAX(10, 100), + }; + int d = 0; + + assert_se(val1.a == 100); + assert_se(MAX(++d, 0) == 1); + assert_se(d == 1); +} + static void test_first_word(void) { assert_se(first_word("Hello", "")); assert_se(first_word("Hello", "Hello")); @@ -926,6 +940,7 @@ int main(int argc, char *argv[]) { test_streq_ptr(); test_align_power2(); + test_max(); test_first_word(); test_close_many(); test_parse_boolean();
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel