On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
<lenn...@poettering.net> wrote:
> On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
>
>> Will result in slightly smaller binaries, and cuts out the branch, even if
>> the expression is still executed.
>
> I am sorry, but the whole point of assert_se() is that it has side
> effects. That's why it is called that way (the "se" suffix stands for
> "side effects"), and is different from assert(). You are supposed to
> use it whenever the code enclosed in it shall not be suppressed if
> NDEBUG is defined. This patch of yours breaks that whole logic!

Hm, am I reading the patch wrong? It looks good to me. It is simply
dropping the branching and logging in the NDEBUG case, but the
expression itself is still evaluated.

So in the NDEBUG case "assert_se(foo())" becomes equivalent to
"foo()", which I guess makes sense (though I doubt it makes much of a
difference).

-t

>> ---
>>  src/shared/macro.h | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/shared/macro.h b/src/shared/macro.h
>> index 7f89951..02219ea 100644
>> --- a/src/shared/macro.h
>> +++ b/src/shared/macro.h
>> @@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long 
>> u) {
>>                  (__x / __y + !!(__x % __y));                            \
>>          })
>>
>> -#define assert_se(expr)                                                 \
>> -        do {                                                            \
>> -                if (_unlikely_(!(expr)))                                \
>> -                        log_assert_failed(#expr, __FILE__, __LINE__, 
>> __PRETTY_FUNCTION__); \
>> -        } while (false)                                                 \
>> -
>>  /* We override the glibc assert() here. */
>>  #undef assert
>>  #ifdef NDEBUG
>> +#define assert_se(expr) do {expr} while(false)
>>  #define assert(expr) do {} while(false)
>>  #else
>> +#define assert_se(expr)                                                 \
>> +        do {                                                            \
>> +                if (_unlikely_(!(expr)))                                \
>> +                        log_assert_failed(#expr, __FILE__, __LINE__, 
>> __PRETTY_FUNCTION__); \
>> +        } while (false)
>>  #define assert(expr) assert_se(expr)
>>  #endif
>>
>> --
>> 2.2.1.209.g41e5f3a
>>
>> _______________________________________________
>> systemd-devel mailing list
>> systemd-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> _______________________________________________
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to