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. _______________________________________________ evolution-hackers mailing list firstname.lastname@example.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers