Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

error handling constants #52

Closed
wants to merge 3 commits into from

3 participants

Eystein Måløy Stenberg Volker Hilsheimer Mikhail Gusarov
Eystein Måløy Stenberg
Owner

Much like the cfapi_errid in Nova.
It is useful if a function may return something else than just true/false.
Normally you have at least three states: true/false and some kind of error (e.g. in ReadDB: false may mean not found or read error).

We could merge the cfapi_errid constants into this one, to have just one error struct.
Thoughts?

Mikhail Gusarov

This approach has the following problems:

  • If function returns cf_errid, then it is potentially returns any of the errors defined in the enum. This works to the some extent in Unix tradition with errno, but only due to the fact every function documents all the errors that could be returned, and the reasons why the errors are returned.

  • handling errors with switch() statement will either have to use "default:" case, which is error-prone, or huge list of case CFERRID_THIS_ONE: FatalError("can't happen").

  • It breaks if(fun()) convention, as the only cf_errid value which evaluates to False is CFERRID_SUCCESS. Negative error codes would help though, if(fun() >= 0) is more-or-less accepted convention.

  • if (fun() == CFERRID_SUCCESS) is ugly.

Volker Hilsheimer

I agree with the problem statement, but I also agree with Mikhail - this is very Microsoft COM-like, which is a synonym for "very easy to get wrong". Unfortunately we can't overload a cast-operator to bool in C.

I'd suggest to split the two commits 52 and 53 - the simplification in 53 seems ok, but the error handling change needs a bit more discussion, I think.

Eystein Måløy Stenberg
Owner

Oki, thanks for the comments.
I like the convention of negative meaning some kind of error, while zero/positive may give a positive status code (e.g. bytes read).

It still is not beautiful if you want to print a message or stop on error.

if((status = SomeFunction()) <= 0)
{
// print error with status
}

There aren't too many options for error handling to choose from in plain C, unfortunately.

I'll try to split 52 an 53 and use some magic error or writing to a param there if I need to for now..
I leave this open until we have agreed on something.

Eystein Måløy Stenberg estenberg closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 26, 2012
  1. Eystein Måløy Stenberg

    error handling constants

    estenberg authored
  2. Eystein Måløy Stenberg
  3. Eystein Måløy Stenberg

    errid for db write

    estenberg authored
This page is out of date. Refresh to see the latest.
Showing with 42 additions and 0 deletions.
  1. +1 −0  src/Makefile.am
  2. +41 −0 src/error.h
1  src/Makefile.am
View
@@ -59,6 +59,7 @@ libpromises_la_SOURCES = \
dtypes.c \
enterprise_stubs.c \
env_context.c \
+ error.h \
evalfunction.c evalfunction.h \
exec_tools.c \
expand.c \
41 src/error.h
View
@@ -0,0 +1,41 @@
+/*
+ Copyright (C) Cfengine AS
+
+ This file is part of Cfengine 3 - written and maintained by Cfengine AS.
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms of the GNU General Public License as published by the
+ Free Software Foundation; version 3.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA
+
+ To the extent this program is licensed as part of the Enterprise
+ versions of Cfengine, the applicable Commerical Open Source License
+ (COSL) may apply to this file if you as a licensee so wish it. See
+ included file COSL.txt.
+*/
+
+#ifndef CFENGINE_ERROR_H
+#define CFENGINE_ERROR_H
+
+
+typedef enum cf_errid
+{
+ CFERRID_SUCCESS,
+ CFERRID_FAILURE,
+ CFERRID_DBM_OPEN,
+ CFERRID_DBM_CLOSE,
+ CFERRID_DBM_WRITE,
+ CFERRID_LOCK_NOT_ACQUIRED,
+ CFERRID_MAX
+} cf_errid;
+
+
+#endif /* NOT CFENGINE_ERROR_H */
Something went wrong with that request. Please try again.