[Pidgin] #4844: [patch] update purple_prefs_* perl bindings
Pidgin
trac at pidgin.im
Thu Feb 14 23:58:38 EST 2008
#4844: [patch] update purple_prefs_* perl bindings
----------------------+-----------------------------------------------------
Reporter: zdeqb | Owner: deryni
Type: patch | Status: new
Priority: minor | Milestone:
Component: plugins | Version: 2.3.1
Resolution: | Keywords: perl prefs patch
Pending: 0 |
----------------------+-----------------------------------------------------
Comment (by zdeqb):
Replying to [comment:1 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.
--
Ticket URL: <http://developer.pidgin.im/ticket/4844#comment:2>
Pidgin <http://pidgin.im>
Pidgin
More information about the Tracker
mailing list