[Swan-dev] Problematic commits in master

[Andrew Cagney]

On 9 April 2017 at 05:50, Tuomo Soini <tis at foobar.fi> wrote:
> These two commits break other development branches where these are used
> heavily. Andrew, could you please revert these for now.

Tuomo,

I'm a little confused.  Are we discussing RETURN_STF_FAILURE_STATUS,
RETURN_STF_FAILURE, or something else?  On which branch is this
causing problems?  I looked at ikev2-fetch-ipseckey?  Should I be
looking elsewhere?

> commit e685c3d6563e1d6611e33a2b58706f94b40821ff
> Author: Andrew Cagney <cagney at gnu.org>
> Date:   Wed Apr 5 09:16:35 2017 -0400
>
>     pluto: inline single use of RETURN_STF_FAILURE_STATUS

Where the macro that was inlined looked like:

#define RETURN_STF_FAILURE_STATUS(f) { \
-       stf_status res = (f); \
-       if (res != STF_OK) { \
-                 return res; \
-       } \

and its sole use was in ikev2_parent.c (merging this might get
oh-so-slightly annoying, but isn't fatal)

I really trust this isn't being used "heavily", having a macro
conditionally "return" as a side effect makes for very confusing code;
but if it is a temporary definition on the branch would be the better
option.

> commit 3f607d178929996931dc56299e2e0816eece2f89
> Author: Andrew Cagney <cagney at gnu.org>
> Date:   Wed Apr 5 09:21:27 2017 -0400
>
>     pluto: delete an unused definition of RETURN_STF_FAILURE

Where the definition:

-#define RETURN_STF_FAILURE(f) { \
-       notification_t res = (f); \
-       if (res != NOTHING_WRONG) { \
-                 return STF_FAIL + res; \
-       } \
-}

was removed from ipsec_dio.c:

- there were zero uses in ipsec_dio.c
- it was likely "wrong", ipsec_dio.c is generic code so doesn't know
if "f()" returns a notification_t or v2_notificatin_t.

The two other definitions:

- ikev2.h which is used "heavily" by ikev2_parent.c (note that macro
is wrong, it should use v2_notification_t)
- ikev1_continuations.h which is used "heavily" by ikev1 code

remain.