Re: [Evolution-hackers] Heads Up: More Camel API breakage in 2.31

[Matthew Barnes <mbarnes redhat com>]

On Sun, 2010-07-11 at 13:08 +0100, David Woodhouse wrote:
> I note that in some places you've elected not to propagate the error
> pointer. For example in imapx_parse_status_info(): 
> 
> -imapx_parse_status_info (struct _CamelIMAPXStream *is, CamelException *ex)
> +imapx_parse_status_info (struct _CamelIMAPXStream *is, GError **error)
> -                               sinfo->messages = camel_imapx_stream_number (is, ex);
> +                               sinfo->messages = camel_imapx_stream_number (is, NULL);
> ... etc.

I already talked to David about this on IRC but also answering here for
posterity.  I elected not to pass a GError where I saw inadequate error
checking in the logic.  In the example above the function was returning
a non-NULL "_state_info" struct whether an error got set or not, and the
calling logic treats a non-NULL return value as successful.  So if a
GError were set there it would probably have leaked.

Another scenario: Camel used to complain with a runtime warning if you
passed NULL to its CamelException functions, presumably to help enforce
proper error checking.  Instead I saw this cute stunt in quite a few
places:

    CamelException ex;

    camel_exception_init (&ex);
    do_something_that_can_fail (folder, message, &ex);
    camel_exception_clear (&ex);

    ... assume success and carry on ...

Those cases got replaced with a NULL GError too.

I wasn't setting out to fix all our bad error handling -- that would've
been too overwhelming.  But I should have at least marked those places
with a FIXME comment so we can grep them easily and go back and clean
them up.  I will reexamine my patches and add comments so those places
aren't lost.