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.
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.
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 PurpleRequestFi eldsCb 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.