Boost logo

Boost :

Subject: Re: [boost] D-style scope guards?
From: Ulrich Eckhardt (doomster_at_[hidden])
Date: 2010-09-19 07:43:58


On Sunday 19 September 2010 10:40:16 Martin Christiansson wrote:
> I need to call a lot of C-functions that use return value and errno
> for error reporting. Writing a lot of code around each call is a bit
> clumsy so I tried to create a guard macro instead that puts all
> relevant data into an exception if the "C function" returned an
> error.

Sounds sensible. Generally though, I would still not use macros for that
unless there's a good reason.

> Now it looks like this example with two small silly C-functions:
>
> extern "C" {
> int foo(const char* s) {
> return strlen(s) - 12;
> }
> const char* bar(int n) {
> return n != 42 ? NULL : "Hello World!";
> }
> }
>
> int main()
> {
> using boost::lambda::_1;
>
> int rc;
> const char* ptr;
>
> try {
> rc = RETVALGUARD(_1 < 0, foo, "Hello World!");
> cout << "rc = " << rc << endl;
>
> ptr = RETVALGUARD(_1 == ((char*) NULL), bar, 42);
> cout << "ptr = " << ptr << endl;
>
> } catch (std::exception& e) {
> cout << (&e)->what() << endl;
> }
>
> return 0;
> }

I'd do this:

  int const x = foo("Hello Christian!");
  if(x < 0)
     throw_errno_exception("foo");
  // use x
  ...

Of course, if you want to pack things like __FILE__ and __LINE__, then you
will need to use macros.

> boost::bind is used when calling the function with it arguments.

Now this is a show-stopper for me, because bind creates a function object
which implies copying the arguments, storing them somewhere, and that
"somewhere" is possibly dynamically allocated, just to throw them away after
the call. This is an insane overhead IMHO, though I'm doing embedded stuff and
being mindful of resources is a must there, sometimes even at the expense of
being able to debug something. YMMV.

What you gain is that you can flexibly define what constitutes failure.
Otherwise, you would need an overloaded function that checks the result,
provided the same returntype always has the same success/failure meaning.

I'd still challenge the advantage of any such solution over writing wrappers:

  namespace wrapped {
     int foo(char const* s) {
        int x = ::foo(s);
        if(x < 0)
           throw_errno_exception("foo");
        return x;
     }
     // ditto for bar()
  }

This has the advantage that you don't repeat the checking code wherever you
need to call foo(). Also, unless you call the original, you won't be able to
forget checking for errors either, as the checking is done in exactly one
place where it also won't clutter your program flow:

  int main() {
     try {
        int const rc = wrapped::foo("Hello World!");
        cout << "rc = " << rc << endl;
 
        char const* ptr = wrapped::bar(42);
        cout << "ptr = " << ptr << endl;
     } catch(std::exception const& e) {
        cout << e.what() << endl;
     }
  }

Here you have zero error checking in the code, just the program logic.

> } catch (std::exception& e) {
> cout << (&e)->what() << endl;
              ^^^^^^
what()? =D

> Still need some code cleanup, but I think it will be quicker to translate
> from C-style return values to a "legacy exception" than writing the code
> manually for each function call.

But that's what you are doing! You _are_ repeating the error checking code for
every call, only that you use a different syntax.

Sorry for shredding your design, I hope that you still gain something from it.

Uli


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk