Created attachment 159124 [details] [review]
Fix GAsyncReadyCallback

Review of attachment 159124 [details] [review]:

Yeah, I figured this would bite is eventually.  There is no perfect way to check that this argument is the callback and is what we're looking for (at least not that Colin and I could come up with @ the hackfest)

I don't entirely understand how your patch tries to fix that though.  You're checking if a null pointer is passed in and in that case using Py_None.  I guess your assumption is that valid pointers will be NULL whereas the callback will be a junk pointer?

(In reply to comment #2)
> Review of attachment 159124 [details] [review]:
> 
> Yeah, I figured this would bite is eventually.  There is no perfect way to
> check that this argument is the callback and is what we're looking for (at
> least not that Colin and I could come up with @ the hackfest)
> 
> I don't entirely understand how your patch tries to fix that though.  You're
> checking if a null pointer is passed in and in that case using Py_None.  I
> guess your assumption is that valid pointers will be NULL whereas the callback
> will be a junk pointer?

Well, this patch just makes sure that if we are passed NULL, we don't try to interpret it as a PyObject* (and crash) and instead pass None.

About the root issue you mention, I cannot say I understand it fully, can you extend on it?

The issue is that we do not know which argument the callback is.  We can't surround the insertion of the callback with a check that the argument is NULL because that could be valid and it may be a case where we need the callback.

(In reply to comment #4)
> The issue is that we do not know which argument the callback is.  We can't
> surround the insertion of the callback with a check that the argument is NULL
> because that could be valid and it may be a case where we need the callback.

I see the difficulties with that, but this patch just makes sure we don't pass NULL as a Python object.

Properly identifying the user_data args should be a separate bug.

The issue is that you're checking if:

if (*((gpointer*)args[i]) == NULL) {


However, inside this case statement the only thing we could be passing to python is

closure->user_data

so I am confused how the check helps at all?

Created attachment 159977 [details] [review]
Fix GAsyncReadyCallback

You are right, sorry for not understanding before. This patch addresses
it and is rebased against HEAD.

Review of attachment 159977 [details] [review]:

Looks good to me!  Thanks for bearing with me with my poor explanations :)

Attachment 159977 [details] pushed as 9fc6783 - Fix GAsyncReadyCallback