[Swan-dev] Problematic commits in master

[Andrew Cagney]

On 12 April 2017 at 03:42, Antony Antony <antony at phenome.org> wrote:
> On Mon, Apr 10, 2017 at 02:10:32PM -0400, Andrew Cagney wrote:
>> Can we agree that the use of macros that conditionally return as a
>> side effect are, in general, a bad idea and their use should not be
>> encouraged?
>
> why is it a bad idea? one reason I can think is running in gdb. I think it is easy to work around that. So I don't agree with you yet.

> When there is a need for several calls to such a macro replacing it with inlined version get confusing.

The behaviour of this code is clear; with or without a debugger it is
easy to understand what is happening; and a debugger will step through
it

> stf_status res = accept_ike_sa_rekey_req(md, pst,st);
> if (res != STF_OK) {
>         return res;
> }
>
> vs

in contrast to this macro which can't be stepped through

> RETURN_STF_FAILURE_STATUS(accept_ike_sa_rekey_req(md, pst,st));
>
> Imagine this 5 - 10 times.
>
> I do find having RETURN_STF_FAILURE and RETURN_STF_FAILURE_STATUS is a bit confusing.
> However, I hope to get rid of one, once we have a general way of replying with notification with payloads.
> My idea is RETURN_STF_FAILURE which return stf_status and not v2_v2_notification_t_t. One day we will get there:)

But we have _no_ occurrences of RETURN_STF_FAILURE_STATUS in master already.