Comment 27 for bug 1070377

Revision history for this message
In , Will Thompson (wjt) wrote :

Comment on attachment 77742
Version 2 of the proposed change

Review of attachment 77742:
-----------------------------------------------------------------

I think this will still leak, actually. Take a look at pidgin/request.c's implementation of request_fields: if the user closes the window without picking a response, it makes sure to call the cancel_cb before purple_request_close(). This allows whatever bit of libpurple made the request to clean up.

But if that's fixed, I think it looks good.

::: src/request.c
@@ +27,3 @@
>
> #include "debug.h"
> #include "request.h"

What happens if libpurple gives us a password request, and before the user answers the challenge, libpurple cancels it?

I think what will happen is: haze_close_request() is called by libpurple, freeing this stuff up.

Then later, haze_request_password_cb() will be called, which will call back into libpurple with a freed request and crash.

@@ +33,2 @@
> static gpointer
> haze_request_input (const char *title,

Use g_slice_free (fields_data, fd);

@@ +104,5 @@
> +struct password_data {
> + PurpleRequestFields *fields;
> + PurpleRequestField *password;
> + GCallback ok_cb;
> + GCallback cancel_cb;

I think it would be better to cast these to PurpleRequestFieldsCb in haze_request_fields, not at the point you call them. I think that would be clearer.

@@ +138,4 @@
> PurpleConversation *conv,
> void *user_data)
> {
> + /*

Use g_slice_new0.

@@ +166,2 @@
>
> return NULL;

I think the idle callback should call the cancel_cb, so that whatever made the request doesn't leak.