watch out with g_return_if_fail (was: r15522 has been commited: )

[Christian Stimming]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Daniel,

thanks for the contribution.

Daniel Espinosa schrieb:
> This is my first work, and can't be stablished as fineshed, I'll do A
> LOT of new changes before to complete the transformation to GObject
> system...

Just a rather small remark that shouldn't get forgotten: You've changed
a bunch of sanity checks into g_return_if_fail() macro calls, but I've
seen several places where you didn't properly switch the arguments
accordingly. Examples below. On IRC we pretty much agreed that
g_return_if_fail() is a rather dangerous function because its semantics
with the "fail" are a bit twisted and you (or anyone else) might easily
get them wrong. Because of that, I (and probably most of us) would
certainly agree if you simply keep on writing  "if (whatever ||
somethingelse) { return; }", because this makes sure the semantics are
still obvious.

Example of erroneous g_return_if_fail:

Modified: gnucash/branches/gobject-engine-dev/lib/libqof/qof/qofbook.c
@@ -272,22 +440,54 @@
   QofCollection *col;

- -  if (!book || !entity_type) return NULL;
+  g_return_val_if_fail (QOF_IS_BOOK (book) || G_TYPE_IS_OBJECT (type),
NULL);

That should have been (QOF_IS_BOOK (book) && G_TYPE_IS_OBJECT (type)).

Or this one:

 void qof_book_set_version (QofBook *book, gint32 version)
 {
- -	if(!book && version < 0) { return; }
+	g_return_if_fail (QOF_IS_BOOK (book) && version < 0);
+	


Should have been (QOF_IS_BOOK (book) && version >= 0).

I'd really recommend you shouldn't use g_return_if_fail except for the
totally simple case when the argument is one single value that is tested
for being non-NULL.

Regards,

Christian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iQCVAwUBRcsgTmXAi+BfhivFAQKpBQQAsI5nrhH4krIPCq0bTpEB0pHrdIKMRKQu
dcvDsSUd6lttnhfvj7kRLLVvI60Ht/4BWzMo7kbm6lhXKo0kExWXrwihB/G/s/tv
1DVtWI6auk6+UJUqLEt7mBA13iELBbYXZK29M6P4sB7dZXcaD4oT3KVwNezi5PHq
NNxV4VkeNrk=
=7+vY
-----END PGP SIGNATURE-----