[Pidgin] #11431: Signals no longer work in Perl plugins

[Pidgin]

#11431: Signals no longer work in Perl plugins
----------------------+-----------------------------------------------------
 Reporter:  dharding  |        Owner:  MarkDoliner
     Type:  defect    |       Status:  new        
Milestone:  2.7.0     |    Component:  plugins    
  Version:  2.6.6     |   Resolution:             
 Keywords:  perl      |  
----------------------+-----------------------------------------------------

Comment(by dharding):

 Replying to [comment:6 MarkDoliner]:
 > dharding: Do you know what the difference is between the perl code and
 purple_status_type_add_attrs_vargs() in libpurple/status.c?  It's not
 clear to me why the latter is ok but the perl function is not.

 Here's my understanding of how things work on x86:  on that platform a
 va_list is just a pointer into the stack indicating the position of the
 next argument.  The va_arg macro dereferences that pointer to get the
 value of that argument, then updates the pointer to point to the next
 argument.

 In libpurple/status.c, va_arg is invoked within a loop in a single
 function that process all the arguments
 (purple_status_set_active_with_attrs or
 purple_status_type_add_attrs_vargs).  In both of those cases, args is a
 local variable that va_arg is able to update, so everything works fine.

 In the Perl code, perl_signal_cb loops over the args, but for each one
 calls purple_perl_sv_from_vargs, which is the function that actually calls
 va_arg.  Consider the case on x86 if a va_list (not a va_list*) is passed
 to purple_perl_sv_from_vargs.  When purple_perl_sv_from_vargs invokes the
 va_arg macro, it updates the local args variable, but it doesn't update
 the args variable in its caller, perl_signal_cb.  Thus on each subsequent
 iteration through the loop, perl_signal_cb will still pass a pointer to
 the first argument to purple_perl_sv_from_vargs, and expected, all sorts
 of weirdness ensues.

 The naive solution is to pass a va_list* to purple_perl_sv_from_vargs, so
 that the changes made by va_arg get seen by the caller, perl_signal_cb, so
 that it can pass the updated pointer on subsequent iterations of the loop.

 Unfortunately, as discussed on the mailing list in the link above and in
 Ticket #7404, on some platforms, including x86-64, va_list is implemented
 as an array, not a pointer.  Because of the way C conflates arrays and
 pointers, this causes problems when trying to take the address of a
 va_list to pass it as a va_list* parameter.

 There are three solutions I know of at this point.  One are to use #ifdefs
 based on whether va_list is an array or not.  This is the change made to
 address Ticket #7404, except that to be correct, it should have used
 #ifdefs around the prototype for purple_perl_sv_from_vargs depending on
 whether it was being passed a va_list or a va_list*, which would also mean
 that purple_perl_sv_from_vargs would need #ifdefs internally whenever
 referencing the args parameter, depending on whether it was a va_list or a
 va_list*.  In short, really messy, ugly code.

 The second is to wrap the va_list in a struct, so that its address can be
 taken in a consistent manner, whether it is a pointer or an array.  This
 results in much cleaner code than using #ifdefs, but requires making an
 extra copy of the va_list at runtime (though this is not likely to have
 any performance implications considering all the marshalling of arguments
 that is being done between Pidgin and Perl).

 The third solution would be to take the code from
 purple_perl_sv_from_vargs and embed it in the loop in perl_signal_cb, so
 that all the calls to va_arg are in a single function.  This would work
 because there are no other callers to purple_perl_sv_from_vargs, but it
 would make code for perl_signal_cb much less clear.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/11431#comment:7>
Pidgin <http://pidgin.im>
Pidgin