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

[Michiel van Baak]

On 17:26, Thu 24 Apr 08, Kevin P. Fleming wrote:
> 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,

Thanks for the feedback.
I'll look into it later. You can revert it for now.

-- 

Michiel van Baak
michiel at vanbaak.eu
http://michiel.vanbaak.eu
GnuPG key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x71C946BD

"Why is it drug addicts and computer aficionados are both called users?"