[asterisk-dev] [asterisk-commits] mvanbaak: trunk r114637 - in /trunk: apps/ channels/ include/asterisk/ main/

[Kevin P. Fleming]

SVN commits to the Asterisk project wrote:
> Author: mvanbaak
> Date: Thu Apr 24 17:16:48 2008
> New Revision: 114637
> 
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=114637
> Log:
> Pass the hangup cause all the way to the calling app/channel.
> 
> (closes issue #11328)
> Reported by: rain
> Patches:
>       20071207__pass_cause_in_hangup_control_frame.diff.txt uploaded by Corydon76 (license 14)
> brought up-to-date to trunk by me

This patch concerns me for at least two reasons:

1) It is reusing the 'seqno' field of the frame structure for something
that is clearly not a sequence number, without documenting this usage in
the header file that defines this structure. This could easily lead to
someone changing the structure definition in the future for sequence
number handling and breaking this code for inexplicable reasons. There
is a 'data' field, we should just use it since we know a hangup cause
will always fit in the space allocated to a 'void *'.

2) It is an API change for ast_queue_hangup(), and we don't yet know how
we are going to handle API changes between 1.6 releases. I certainly
don't want to cause out-of-tree code to no longer compile just because
someone upgraded from 1.6.0 to 1.6.1, especially when preserving
backwards compatibility would have been so easy to do. Please consider
restoring ast_queue_hangup() to its old arguments and adding
ast_queue_hangup_with_cause() for the new version.

-- 
Kevin P. Fleming
Director of Software Technologies
Digium, Inc. - "The Genuine Asterisk Experience" (TM)