Merge lp:~henrik-ziegeldorf/hipl/HIP_EAE into lp:hipl

[Henrik Ziegeldorf]

This branch introduces the HIP_EAE macro and replaces occurrences of HIP_IFEL(1,x,x) with HIP_EAE(x,x).

HIP_EAE(eval, args, ...) prints an error message, sets err to eval and jumps to out_err.
It is substantially different from HIP_IFE(cond, eval) which evaluates a condition and if true, sets err to eval and jumps to out_err. Essentially, HIP_IFE is HIP_IFEL without an error message.

Replacing HIP_IFEL(1,x,x) constructs with HIP_EAE mainly contributes to code-readability.

[Diego Biurrun (diego-biurrun)]

 review: needs-fixing

On Sat, Jan 15, 2011 at 12:04:09PM +0000, Henrik Ziegeldorf wrote:
>
> Requested reviews:
> HIPL core team (hipl-core)
>
> For more details, see:
> https://code.launchpad.net/~henrik-ziegeldorf/hipl/HIP_EAE/+merge/46381
>
> This branch introduces the HIP_EAE macro and replaces occurrences of HIP_IFEL(1,x,x) with HIP_EAE(x,x).
>
> HIP_EAE(eval, args, ...) prints an error message, sets err to eval and jumps to out_err.
> It is substantially different from HIP_IFE(cond, eval) which evaluates a condition and if true, sets err to eval and jumps to out_err. Essentially, HIP_IFE is HIP_IFEL without an error message.
>
> Replacing HIP_IFEL(1,x,x) constructs with HIP_EAE mainly contributes to code-readability.

Good idea in general, but I still passionately hate those ugly macros.
At least future code should switch to a more sensible error handling
strategy without gotos in all cases.

A somewhat related general remark: The macros from ife.h are quite
similar and could/should be refactored to avoid the code duplication.

> --- hipd/output.c 2011-01-12 13:06:31 +0000
> +++ hipd/output.c 2011-01-15 12:04:09 +0000
> @@ -459,7 +459,7 @@
> default:
> - HIP_IFEL(1, -ENOSYS, "HIP transform not supported (%d)\n",
> + HIP_EAE(-ENOSYS, "HIP transform not supported (%d)\n",
> transform_hip_suite);

indent

> --- lib/core/ife.h 2011-01-10 13:34:58 +0000
> +++ lib/core/ife.h 2011-01-15 12:04:09 +0000
> @@ -34,6 +34,23 @@
>
> /**
> + * Use this macro to exit a function and output an error message.
> + * Variable 'err' must be defined, usually type int.
> + * Label 'out_err' must be defined, on errors this label is used
> + * as destination after proper actions.
> + *
> + * @param eval Set variable called 'err' to this value.

I think "errorcode" describes this better.

> + * @note EAE stands for ERROR and EXIT.

If you need to explain it, the name is rather poor. Note that I had
wondered what HIP_EAE stood for before reading this comment.

> +#define HIP_EAE(eval, args ...) \
> + { \
> + HIP_ERROR(args); \
> + err = eval; \
> + goto out_err; \
> + }

I wonder if the extra { } are necessary.

Diego

review: Needs Fixing

[lp:~henrik-ziegeldorf/hipl/HIP_EAE]

5525. By Henrik Ziegeldorf on 2011-01-15

Fixed indentation.

[Henrik Ziegeldorf (henrik-ziegeldorf)]

> review: needs-fixing
>
> On Sat, Jan 15, 2011 at 12:04:09PM +0000, Henrik Ziegeldorf wrote:
> >
> > Requested reviews:
> > HIPL core team (hipl-core)
> >
> > For more details, see:
> > https://code.launchpad.net/~henrik-ziegeldorf/hipl/HIP_EAE/+merge/46381
> >
> > This branch introduces the HIP_EAE macro and replaces occurrences of
> HIP_IFEL(1,x,x) with HIP_EAE(x,x).
> >
> > HIP_EAE(eval, args, ...) prints an error message, sets err to eval and jumps
> to out_err.
> > It is substantially different from HIP_IFE(cond, eval) which evaluates a
> condition and if true, sets err to eval and jumps to out_err. Essentially,
> HIP_IFE is HIP_IFEL without an error message.
> >
> > Replacing HIP_IFEL(1,x,x) constructs with HIP_EAE mainly contributes to
> code-readability.
>
> Good idea in general, but I still passionately hate those ugly macros.

Yes we know that by now. So, you don't want this macro, just reject the merge proposal.

One reason I introduced this branch was, that there didn't seem to be a consensus about introducing this macro, but people kept commenting HIP_IFEL(1,x,x) constructs in my ECC branch. So this is my way of pushing for decision.

> At least future code should switch to a more sensible error handling
> strategy without gotos in all cases.

I think we should either abandon this completely or stick to this style.
Having old code with those macros and new code without them seems wrong to me. Especially since it is easy to just replace all those macros with the if (err) { ...; goto out_err;} code you long for so much.

>
> > --- hipd/output.c 2011-01-12 13:06:31 +0000
> > +++ hipd/output.c 2011-01-15 12:04:09 +0000
> > @@ -459,7 +459,7 @@
> > default:
> > - HIP_IFEL(1, -ENOSYS, "HIP transform not supported (%d)\n",
> > + HIP_EAE(-ENOSYS, "HIP transform not supported (%d)\n",
> > transform_hip_suite);
>
> indent

Fixed.

>
> > + * @note EAE stands for ERROR and EXIT.
>
> If you need to explain it, the name is rather poor. Note that I had
> wondered what HIP_EAE stood for before reading this comment.
>

Well, then it would be nice if you proposed a better name.

> > +#define HIP_EAE(eval, args ...) \
> > + { \
> > + HIP_ERROR(args); \
> > + err = eval; \
> > + goto out_err; \
> > + }
>
> I wonder if the extra { } are necessary.
>

No, they're not. Nevertheless I'm hesitant about removing them. Does it improve readability?

[Diego Biurrun (diego-biurrun)]

On Sat, Jan 15, 2011 at 01:12:52PM +0000, Henrik Ziegeldorf wrote:
> > review: needs-fixing
> >
> > On Sat, Jan 15, 2011 at 12:04:09PM +0000, Henrik Ziegeldorf wrote:
> > >
> > > Requested reviews:
> > > HIPL core team (hipl-core)
> > >
> > > For more details, see:
> > > https://code.launchpad.net/~henrik-ziegeldorf/hipl/HIP_EAE/+merge/46381
> > >
> > > This branch introduces the HIP_EAE macro and replaces occurrences of
> > HIP_IFEL(1,x,x) with HIP_EAE(x,x).
> > >
> > > HIP_EAE(eval, args, ...) prints an error message, sets err to eval and jumps
> > to out_err.
> > > It is substantially different from HIP_IFE(cond, eval) which evaluates a
> > condition and if true, sets err to eval and jumps to out_err. Essentially,
> > HIP_IFE is HIP_IFEL without an error message.
> > >
> > > Replacing HIP_IFEL(1,x,x) constructs with HIP_EAE mainly contributes to
> > code-readability.
> >
> > Good idea in general, but I still passionately hate those ugly macros.
>
> Yes we know that by now. So, you don't want this macro, just reject the merge proposal.

Give a man some room to rant every once in a while :)

> One reason I introduced this branch was, that there didn't seem to be
> a consensus about introducing this macro, but people kept commenting
> HIP_IFEL(1,x,x) constructs in my ECC branch. So this is my way of
> pushing for decision.

I'm in favor.

> > At least future code should switch to a more sensible error handling
> > strategy without gotos in all cases.
>
> I think we should either abandon this completely or stick to this style.
> Having old code with those macros and new code without them seems wrong to me.

We already have functions that return errors through "return" instead
of "goto", so I don't see a problem. Sticking to a style is good as
long as that style is sensible. If not, it makes sense to move away
from the established practice in the medium or long term.

> Especially since it is easy to just replace all those macros with the
> if (err) { ...; goto out_err;} code you long for so much.

I do not long for making all those gotos explicit. However, I believe
that in many cases, functions can be rewritten to just return directly
in case of error and be simplified in the process.

I'll see if I can dig up an example.

> > > + * @note EAE stands for ERROR and EXIT.
> >
> > If you need to explain it, the name is rather poor. Note that I had
> > wondered what HIP_EAE stood for before reading this comment.
>
> Well, then it would be nice if you proposed a better name.

Hah, good names are not so easy :)

HIP_ERR_EXIT or HIP_ERROR_EXIT

> > > +#define HIP_EAE(eval, args ...) \
> > > + { \
> > > + HIP_ERROR(args); \
> > > + err = eval; \
> > > + goto out_err; \
> > > + }
> >
> > I wonder if the extra { } are necessary.
>
> No, they're not. Nevertheless I'm hesitant about removing them. Does it improve readability?

I would think so.

However, the following needs no braces in the first place:

  #define HIP_EAE(eval, args ...) HIP_IFEL(1, eval, args ...)

The compiler should optimize away the pointless if-conditional.

Diego

[Diego Biurrun (diego-biurrun)]

On Sat, Jan 15, 2011 at 03:27:59PM +0100, Diego Biurrun wrote:
> On Sat, Jan 15, 2011 at 01:12:52PM +0000, Henrik Ziegeldorf wrote:
> > >
> > > At least future code should switch to a more sensible error handling
> > > strategy without gotos in all cases.
> >
> > I think we should either abandon this completely or stick to this style.
> > Having old code with those macros and new code without them seems wrong to me.
> > Especially since it is easy to just replace all those macros with the
> > if (err) { ...; goto out_err;} code you long for so much.
>
> I do not long for making all those gotos explicit. However, I believe
> that in many cases, functions can be rewritten to just return directly
> in case of error and be simplified in the process.
>
> I'll see if I can dig up an example.

Here is an example from hipd/maintenance.c. It digs up a *very* stupid
mistake, but they are glaringly obvious in plain C while nobody ever
noticed them in the CPP version.

I'll paste the original here and then send my version in a reply. Let's
see who can spot the error and how long it takes to spot the error.

  /**
   * deliver pending retransmissions for all host associations
   *
   * @return zero on success or negative on failure
   */
  static int hip_scan_retransmissions(void)
  {
      int err = 0;
      time_t current_time;
      time(&current_time);
      HIP_IFEL(hip_for_each_ha(hip_handle_retransmission, &current_time), 0,
               "for_each_ha err.\n");
  out_err:
      return err;
  }

Diego

[Diego Biurrun (diego-biurrun)]

On Sat, Jan 15, 2011 at 03:53:34PM +0100, Diego Biurrun wrote:
> On Sat, Jan 15, 2011 at 03:27:59PM +0100, Diego Biurrun wrote:
> > On Sat, Jan 15, 2011 at 01:12:52PM +0000, Henrik Ziegeldorf wrote:
> > > >
> > > > At least future code should switch to a more sensible error handling
> > > > strategy without gotos in all cases.
> > >
> > > I think we should either abandon this completely or stick to this style.
> > > Having old code with those macros and new code without them seems wrong to me.
> > > Especially since it is easy to just replace all those macros with the
> > > if (err) { ...; goto out_err;} code you long for so much.
> >
> > I do not long for making all those gotos explicit. However, I believe
> > that in many cases, functions can be rewritten to just return directly
> > in case of error and be simplified in the process.
> >
> > I'll see if I can dig up an example.
>
> Here is an example from hipd/maintenance.c. It digs up a *very* stupid
> mistake, but they are glaringly obvious in plain C while nobody ever
> noticed them in the CPP version.
>
> I'll paste the original here and then send my version in a reply. Let's
> see who can spot the error and how long it takes to spot the error.
>
> /**
> * deliver pending retransmissions for all host associations
> *
> * @return zero on success or negative on failure
> */
> static int hip_scan_retransmissions(void)
> {
> int err = 0;
> time_t current_time;
> time(&current_time);
> HIP_IFEL(hip_for_each_ha(hip_handle_retransmission, &current_time), 0,
> "for_each_ha err.\n");
> out_err:
> return err;
> }

  /**
   * deliver pending retransmissions for all host associations
   *
   * @return zero on success or negative on failure
   */
  static int hip_scan_retransmissions(void)
  {
      time_t current_time;
      time(&current_time);
      if (hip_for_each_ha(hip_handle_retransmission, &current_time)) {
          HIP_ERROR("for_each_ha err.\n");
          return 0;
      }
      return 0;
  }

I do not think it is a coincidence that this mistake went unnoticed with
HIP_IFEL, do you? Oh, the same error was copypasted into a very similar
function 10 lines above...

The only sensible thing to do is to validate *all* uses of the macro,
I'm convinced that many errors will be uncovered...

Diego

[Stefan Götz (stefan.goetz-deactivatedaccount)]

> > I think we should either abandon this completely or stick to this style.
> > Having old code with those macros and new code without them seems wrong to
> me.
>
> We already have functions that return errors through "return" instead
> of "goto", so I don't see a problem. Sticking to a style is good as
> long as that style is sensible. If not, it makes sense to move away
> from the established practice in the medium or long term.

While this discussion may be worthwhile, it is IMO off-topic for this merge proposal.

> > Well, then it would be nice if you proposed a better name.
>
> Hah, good names are not so easy :)
>
> HIP_ERR_EXIT or HIP_ERROR_EXIT

I'd suggest HIP_OUT_ERR() for two reasons. 1) 'exit' is often associated with terminating the current process (thanks to the exit() function that does exactly that) 2) 'HIP_OUT_ERR()' correlates with the label 'out_err' (which, I hope, was obvious because that's kind of the point ;-)

> #define HIP_EAE(eval, args ...) HIP_IFEL(1, eval, args ...)

+1

review: Needs Fixing

[René Hummen (rene-hummen)]

> #define HIP_EAE(eval, args ...) HIP_IFEL(1, eval, args ...)
>
> The compiler should optimize away the pointless if-conditional.

I don't see the need for yet another macro:
1.) As pointed out by Diego, HIP_IFEL(1, eval, args ...) will be optimized by the compiler.
2.) I don't think there will be a benefit in terms of saving developer brain cycles.

[Diego Biurrun (diego-biurrun)]

On Tue, Jan 18, 2011 at 05:25:46PM +0000, René Hummen wrote:
> > #define HIP_EAE(eval, args ...) HIP_IFEL(1, eval, args ...)
> >
> > The compiler should optimize away the pointless if-conditional.
>
> I don't see the need for yet another macro:
> 1.) As pointed out by Diego, HIP_IFEL(1, eval, args ...) will be optimized by the compiler.
> 2.) I don't think there will be a benefit in terms of saving developer brain cycles.

This HIP_IFEL(1, ...) trickery is maybe acceptable in the implementation
of HIP_OUT_ERR, but littering the code with it I consider a criminal
offense ;)

Trying to avoid HIP_IFEL for all sorts of error handling should be fine,
but avoiding HIP_OUT_ERR, which is at least suitable named, why?

While we're at it, can anybody explain what IFEL in HIP_IFEL stands for?

Diego

[Miika Komu (miika-iki)]

I am not convinced that we need the new macro. However, I think the current naming convention is very intuitive (may be origin of the macro is "IF Error Label", don't remember).

[Diego Biurrun (diego-biurrun)]

On Wed, Jan 19, 2011 at 10:21:13AM +0000, Miika Komu wrote:
> I am not convinced that we need the new macro.

OMG, the defenders of HIP_IFEL(1, ...) crawling out of the woodwork ;)

> However, I think the current naming convention is very intuitive (may
> be origin of the macro is "IF Error Label", don't remember).

Sorry, but no, this is not true, it is not intuitive at all, you
just know what it means because you wrote it and/or whoever wrote
it told you what it means. :)

I talked to Henrik about this yesterday. We have both been working on
HIPL for some time now and we never figured out what it was supposed to
mean and both mentioned that we are still wondering what it could be.

I think this is proof enough that it is not intuitive.

Diego

[lp:~henrik-ziegeldorf/hipl/HIP_EAE]

5526. By Henrik Ziegeldorf on 2011-01-24

Renamed HIP_EAE to HIP_OUT_ERR.

5527. By Henrik Ziegeldorf on 2011-01-24

Removed outdated note from documentation of HIP_OUT_ERR.

5528. By Henrik Ziegeldorf on 2011-02-03

Merged trunk revision 5582.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

HIP_EAE renamed to HIP_OUT_ERR -> approve

review: Approve

[Stefan Götz (stefan.goetz-deactivatedaccount)]

Oops, sorry: approve -> needs fixing, because I'm still waiting for the following change brought up by Diego:

#define HIP_EAE(eval, args ...) HIP_IFEL(1, eval, args ...)

review: Needs Fixing

[Henrik Ziegeldorf (henrik-ziegeldorf)]

> Oops, sorry: approve -> needs fixing, because I'm still waiting for the
> following change brought up by Diego:
>
> #define HIP_EAE(eval, args ...) HIP_IFEL(1, eval, args ...)

This is does not compile:
e.g. hipd/cert.c:188: error: expected ‘)’ before ‘...’ token

[Stefan Götz (stefan.goetz-deactivatedaccount)]

The correct syntax is described in the CPP docs http://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html , so this should be better (haven't tried it though):

#define HIP_OUT_ERR(err, ...) HIP_IFEL(1, err, ##__VA_ARGS__)

[lp:~henrik-ziegeldorf/hipl/HIP_EAE]

5529. By Henrik Ziegeldorf on 2011-02-03

Definition of HIP_OUT_ERR using HIP_IFEL.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

Cool :-) -> Approve

Diego: do you concur? If so, could you change your vote to 'Approve', too, to unblock the merge?

review: Approve

[Diego Biurrun (diego-biurrun)]

On Fri, Feb 04, 2011 at 10:27:58AM +0000, Stefan Götz wrote:
> Review: Approve
> Cool :-) -> Approve
>
> Diego: do you concur? If so, could you change your vote to 'Approve', too, to unblock the merge?

Yes. I don't have my lp login until Monday, feel free to commit
earlier without my lp approval stamp...

Diego

=== modified file 'firewall/rule_management.c'
--- firewall/rule_management.c	2011-01-14 17:01:00 +0000
+++ firewall/rule_management.c	2011-02-03 18:14:50 +0000
@@ -489,7 +489,7 @@
         HIP_IFEL(load_dsa_file(fp, hi),     -1, "Failed to load DSA key\n")
         break;
     default:
-        HIP_IFEL(1, -1, "Could not load host identity, because algorithm is unknown.\n");
+        HIP_OUT_ERR(-1, "Could not load host identity, because algorithm is unknown.\n");
     }
 
     /* verify hi => hit */

=== modified file 'hipd/cert.c'
--- hipd/cert.c	2011-01-31 19:54:26 +0000
+++ hipd/cert.c	2011-02-03 18:14:50 +0000
@@ -185,7 +185,7 @@
         sig_len = SHA_DIGEST_LENGTH + DSA_PRIV * 2;
         break;
     default:
-        HIP_IFEL(1, -1, "Unknown algorithm for signing\n");
+        HIP_OUT_ERR(-1, "Unknown algorithm for signing\n");
     }
 
     /* clearing signature field just to be sure */
@@ -264,7 +264,7 @@
                 p_b64, q_b64, g_b64, y_b64);
         break;
     default:
-        HIP_IFEL(1, -1, "Unknown algorithm for public-key element\n");
+        HIP_OUT_ERR(-1, "Unknown algorithm for public-key element\n");
     }
 
     /* Put the results into the msg back */
@@ -534,7 +534,7 @@
         evpret = EVP_DecodeBlock(y_bin, y_b64, strlen((char *) y_b64));
         break;
     default:
-        HIP_IFEL(1, -1, "Unknown algorithm\n");
+        HIP_OUT_ERR(-1, "Unknown algorithm\n");
     }
 
     memset(sha_digest, '\0', sizeof(sha_digest));
@@ -603,7 +603,7 @@
         HIP_IFEL((err = err == 1 ? 0 : -1), -1, "DSA_do_verify error\n");
         break;
     default:
-        HIP_IFEL(1, -1, "Unknown algorithm\n");
+        HIP_OUT_ERR(-1, "Unknown algorithm\n");
     }
 
     hip_msg_init(msg);
@@ -821,7 +821,7 @@
                  "Failed to set public key of the certificate\n");
         break;
     default:
-        HIP_IFEL(1, -1, "Unknown algorithm\n");
+        HIP_OUT_ERR(-1, "Unknown algorithm\n");
     }
 
     if (sec_ext != NULL) {
@@ -907,7 +907,7 @@
         digest = EVP_dss1();
         break;
     default:
-        HIP_IFEL(1, -1, "Unknown algorithm\n");
+        HIP_OUT_ERR(-1, "Unknown algorithm\n");
     }
 
     HIP_IFEL(!X509_sign(cert, pkey, digest), -1,

=== modified file 'hipd/cookie.c'
--- hipd/cookie.c	2011-01-14 15:53:02 +0000
+++ hipd/cookie.c	2011-02-03 18:14:50 +0000
@@ -402,7 +402,7 @@
         signature_func = hip_dsa_sign;
         break;
     default:
-        HIP_IFEL(1, -1, "Unkown algorithm");
+        HIP_OUT_ERR(-1, "Unkown algorithm");
     }
 
     HIP_IFE(!hip_precreate_r1(entry->r1, &entry->lhi.hit,

=== modified file 'hipd/hadb.c'
--- hipd/hadb.c	2011-01-21 13:22:09 +0000
+++ hipd/hadb.c	2011-02-03 18:14:50 +0000
@@ -978,7 +978,7 @@
         entry->peer_pub_key = hip_key_rr_to_dsa((struct hip_host_id_priv *) entry->peer_pub, 0);
         break;
     default:
-        HIP_IFEL(1, -1, "Unkown algorithm");
+        HIP_OUT_ERR(-1, "Unkown algorithm");
     }
 
 out_err:

=== modified file 'hipd/nat.c'
--- hipd/nat.c	2011-01-11 13:59:46 +0000
+++ hipd/nat.c	2011-02-03 18:14:50 +0000
@@ -229,7 +229,7 @@
         break;
     default:
         err = -1;
-        HIP_IFEL(1, -1, "Unknown nat mode %d\n", nat_mode);
+        HIP_OUT_ERR(-1, "Unknown nat mode %d\n", nat_mode);
     }
     HIP_IFEL(hip_for_each_ha(hip_ha_set_nat_mode, &nat), 0,
              "Error from for_each_ha().\n");

=== modified file 'hipd/output.c'
--- hipd/output.c	2011-01-21 13:08:21 +0000
+++ hipd/output.c	2011-02-03 18:14:50 +0000
@@ -454,8 +454,8 @@
                              sizeof(struct hip_encrypted_null_sha1);
             break;
         default:
-            HIP_IFEL(1, -ENOSYS, "HIP transform not supported (%d)\n",
-                     transform_hip_suite);
+            HIP_OUT_ERR(-ENOSYS, "HIP transform not supported (%d)\n",
+                        transform_hip_suite);
         }
     } else {   /* add host id in plaintext without encrypted wrapper */
                /* Parameter HOST_ID. Notice that hip_get_public_key overwrites

=== modified file 'lib/core/builder.c'
--- lib/core/builder.c	2011-01-31 20:21:37 +0000
+++ lib/core/builder.c	2011-02-03 18:14:50 +0000
@@ -3831,7 +3831,7 @@
                  "key_rr_len\n");
         break;
     default:
-        HIP_IFEL(1, -1, "Unknown algorithm\n");
+        HIP_OUT_ERR(-1, "Unknown algorithm\n");
     }
 
     if (is_public) {

=== modified file 'lib/core/conf.c'
--- lib/core/conf.c	2011-02-03 14:36:53 +0000
+++ lib/core/conf.c	2011-02-03 18:14:50 +0000
@@ -1415,7 +1415,7 @@
         HIP_INFO("Displaying no debugging messages\n");
         status = HIP_MSG_SET_DEBUG_NONE;
     } else {
-        HIP_IFEL(1, -EINVAL, "Unknown argument\n");
+        HIP_OUT_ERR(-EINVAL, "Unknown argument\n");
     }
 
     HIP_IFEL(hip_build_user_hdr(msg, status, 0),
@@ -1555,7 +1555,7 @@
     } else if (!strcmp("get", opt[0])) {
         status = HIP_MSG_LOCATOR_GET;
     } else {
-        HIP_IFEL(1, -1, "bad args\n");
+        HIP_OUT_ERR(-1, "bad args\n");
     }
     HIP_IFEL(hip_build_user_hdr(msg, status, 0), -1,
              "Failed to build user message header.: %s\n", strerror(err));
@@ -2181,7 +2181,7 @@
     } else if (!strcmp("off", opt[0])) {
         status = HIP_MSG_NSUPDATE_OFF;
     } else {
-        HIP_IFEL(1, -1, "bad args\n");
+        HIP_OUT_ERR(-1, "bad args\n");
     }
     HIP_IFEL(hip_build_user_hdr(msg, status, 0), -1,
              "Failed to build user message header.: %s\n", strerror(err));

=== modified file 'lib/core/crypto.c'
--- lib/core/crypto.c	2011-01-17 20:21:16 +0000
+++ lib/core/crypto.c	2011-02-03 18:14:50 +0000
@@ -453,7 +453,7 @@
         break;
 
     default:
-        HIP_IFEL(1, -EINVAL, "Attempted to use unknown CI (alg = %d)\n", alg);
+        HIP_OUT_ERR(-EINVAL, "Attempted to use unknown CI (alg = %d)\n", alg);
     }
 
     err = 0;

=== modified file 'lib/core/ife.h'
--- lib/core/ife.h	2011-01-10 13:34:58 +0000
+++ lib/core/ife.h	2011-02-03 18:14:50 +0000
@@ -34,6 +34,16 @@
  */
 
 /**
+ * Use this macro to exit a function and output an error message.
+ * Variable 'err' must be defined, usually type int.
+ * Label 'out_err' must be defined, on errors this label is used
+ * as destination after proper actions.
+ *
+ * @param eval Set variable called 'err' to this value.
+ */
+#define HIP_OUT_ERR(eval, args ...) HIP_IFEL(1, err, args)
+
+/**
  * Use this macro to detect failures and exit function in case
  * of such. Variable 'err' must be defined, usually type int.
  * Label 'out_err' must be defined, on errors this label is used

=== modified file 'lib/core/solve.c'
--- lib/core/solve.c	2011-01-11 13:59:46 +0000
+++ lib/core/solve.c	2011-02-03 18:14:50 +0000
@@ -98,7 +98,7 @@
         maxtries = 1ULL << (u->pz.K + 3);
         get_random_bytes(&randval, sizeof(uint64_t));
     } else {
-        HIP_IFEL(1, 0, "Unknown mode: %d\n", mode);
+        HIP_OUT_ERR(0, "Unknown mode: %d\n", mode);
     }
 
     HIP_DEBUG("K=%u, maxtries (with k+2)=%llu\n", u->pz.K, maxtries);