Re: Ball of string -> g_return_* macros in static functions - lots of them :(

[John Rice <John Rice Sun COM>]

Kalle,

The problem is we now have a mixture of code using the g_return_* calls as they are meant to in public functions to validate input, but we also have g_return_* being used in static functions to validate some internal state. There's nothing wrong in a programmer validating the internal state of his library at any time, but he should be using g_assert to do so.

If they don't use g_assert then we have to turn off all the checking in the library to get the performance gain, but now our external library calls don't have any parameter validation in them :(

Kinda sucks - and yes you are entirely correct that we could just remove the g_return_* calls from all static functions, but I'd have thought converting them into g_asserts was probably more consistent with the original authors intention - though could be wrong of course :)

JR

Snip from Frederico: http://www.mail-archive.com/desktop-devel-list gnome org/msg02548.html

http://developer.gnome.org/doc/guides/programming-guidelines/robust.html

See the "Assertions and Preconditions" section on that page.

Summary:  use g_return_*() in library entry points to ensure that no
garbage is fed into your library.  Use g_assert() internally to ensure
that you are in a consistent state.

Kalle Vahlman wrote:

2005/10/17, John Rice <John Rice sun com>:
  

So with this situation we will always pay the performance penalty AND we
don't have the benefit of assert's blowing up in the internal library
static functions to warn us during development that something is
screwed. The g_return_* macros will allow us to fail gracefully,
essentially masking many pathological conditions that we really ought to
catch.
    

For developement, you can turn those warnings into fatal (aborting the program).

For performance, you can do -DG_DISABLE_CHECKS because:

http://cvs.gnome.org/viewcvs/glib/glib/gmessages.h?view=markup
---8<-----

#ifdef G_DISABLE_CHECKS

#define g_return_if_fail(expr)                  G_STMT_START{ (void)0;
}G_STMT_END
#define g_return_val_if_fail(expr,val)          G_STMT_START{ (void)0;
}G_STMT_END
#define g_return_if_reached()                   G_STMT_START{ return;
}G_STMT_END
#define g_return_val_if_reached(val)            G_STMT_START{ return
(val); }G_STMT_END

#else /* !G_DISABLE_CHECKS */

---8>----

You shouldn't use g_return_* as a validator of data anyway, they are
more debugging tools then a failsafe mechanism. If you pass crappy
data to functions, there is only so much the functions can do about
it...

If there is something special in using these macros in static
functions, please correct me, but as such I don't see benefits from
turning them into assertions.

--
Kalle Vahlman, zuh iki fi
Powered by http://movial.fi
_______________________________________________
Performance-list mailing list
Performance-list gnome org
http://mail.gnome.org/mailman/listinfo/performance-list