On 9/26/06, Richard Laager <rlaager@wiktel.com> wrote:
I've changed this to a g_return_if_fail(), and added a similar check to
yahoo_buddy_icon_upload(). Here's my commit message, which I think sums
everything up:

After the comments on gaim-devel about 'bad' code, I looked into the
function referenced. It appears that yahoo_buddy_icon_upload_connected()
is only called as a callback. It's registered from
yahoo_buddy_icon_upload(). yahoo_buddy_icon_upload() is only called
twice. In neither case can a NULL make it down to
yahoo_buddy_icon_upload_connected(). As this is an exceptional case
rather than a normal one, a g_return_if_fail() is more appropriate. That
adds logging, makes the intent clearer, and will help the Coverity
software detect violations of this assumption.

I don't remember why I have that check there. Perhaps I did call it with NULL or some point, or was just being overly paranoid. Or maybe I wasn't thinking clearly, and erroneously thought a not null check would save me from invalid memory. Looking at it now, it probably does make more sense as a g_return_if_fail().