[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