[patch] update purple_prefs_* perl bindings

[zdeqb]

This patch adds perl bindings for all the missing functions from prefs.h, specifically:

purple_prefs_{get,set,add}_path(), purple_prefs_{get,set,add}_path_list(), purple_prefs_disconnect_callback(), purple_prefs_get_children_names(), purple_prefs_disconnect_by_handle()

It also adds support for using purple_prefs_connect_callback() from perl.

[deryni]

Cna you tell me why you used mXPUSH* functions for the first three arguments to the pref callback function but XPUSHs for the last one? Is there a reason you capture the return value of call_sv but then don't use it anywhere? Passing the result of g_strdup to a function expecting a const gchar * is going to leak memory (in purple_perl_prefs_connect_callback). Is SvREFCNT_dec enough to free up the result of newSVsv and purple_perl_bless_object? (It might be, I haven't looked at the perl stuff in a while so I don't remember offhand.)

Thanks for the patch, I'll accept the patch once my comments/questions above are taken care of, I'd handle some of them but at the moment I don't have time to dig into it, so I would appreciate it if you could.

[zdeqb]

Replying to deryni:

Cna you tell me why you used mXPUSH* functions for the first three arguments to the pref callback function but XPUSHs for the last one?

perlguts states:

Despite their suggestions in earlier versions of this document the macros (X)PUSH[iunp] are not suited to XSUBs which return multiple results. For that, either stick to the (X)PUSHs macros shown above, or use the new m(X)PUSH[iunp] macros instead; see "Putting a C value on Perl stack".

I could change the mXPUSHp() to XPUSHp(), and the first mXPUSHi() to XPUSHi() but I thought I'd be consistent, and use only mXPUSH*.

Is there a reason you capture the return value of call_sv but then don't use it anywhere?

Since it returns something, I thought I'd save it ;) The return value will always be 0 since call_sv() was used with G_VOID. Since it's not needed I've removed it.

Passing the result of g_strdup to a function expecting a const gchar * is going to leak memory (in purple_perl_prefs_connect_callback).

Fixed it :)

Is SvREFCNT_dec enough to free up the result of newSVsv and purple_perl_bless_object? (It might be, I haven't looked at the perl stuff in a while so I don't remember offhand.)

SvREFCNT_dev() gets rid of our reference to the SV*, and than sv_free()'s it when it reaches 0, so it should be enough

Thanks for the patch, I'll accept the patch once my comments/questions above are taken care of, I'd handle some of them but at the moment I don't have time to dig into it, so I would appreciate it if you could.

[zdeqb]

try #2

[zdeqb]

removes useless code

[zdeqb]

uploaded a new patch, after consulting perlcall again, which removes some not-needed macros, since no value is returned by purple_perl_pref_connect_callback()

[zdeqb]

This ticket can be closed, since code for purple_prefs_connect_callback usage from perl got commited... (which was the main reason for this patch)

[deryni]

I'm leaving this open since it adds functions that the other patch does not.

[datallah]

I wish I had seen this before implementing the purple_perl_pref_connect_callback() stuff.

[datallah@…]

(In [f530f6e0e82a6a0e7b2a19dd0715a4e492dfd177]):
Another Perl loader patch from Zsombor Welker, this one adds some missing pref related functions. Fixes #4844