[Home]

Summary:ASTERISK-14740: chan_sip.c : SIP_PAGE2_CALL_ONHOLD* flags missing a bit
Reporter:pherman (pherman)Labels:
Date Opened:2009-08-31 08:31:34Date Closed:2011-06-07 14:07:28
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:here's an easy one.  In chan_sip.c, we have:

#define SIP_PAGE2_CALL_ONHOLD           (3 << 23)       /*!< D: Call hold states: */
#define SIP_PAGE2_CALL_ONHOLD_ACTIVE    (1 << 23)       /*!< D: Active hold */
#define SIP_PAGE2_CALL_ONHOLD_ONEDIR    (2 << 23)       /*!< D: One directional hold */
#define SIP_PAGE2_CALL_ONHOLD_INACTIVE  (3 << 23)       /*!< D: Inactive hold */

I'm not aware of any bugs this may cause, but this can't be correct, can it?  I suggest:

#define SIP_PAGE2_CALL_ONHOLD           (7 << 22)       /*!< D: Call hold states: */
#define SIP_PAGE2_CALL_ONHOLD_ACTIVE    (1 << 22)       /*!< D: Active hold */
#define SIP_PAGE2_CALL_ONHOLD_ONEDIR    (2 << 22)       /*!< D: One directional hold */
#define SIP_PAGE2_CALL_ONHOLD_INACTIVE  (4 << 22)       /*!< D: Inactive hold */

and moving the other flags down one bit.
Comments:By: David Vossel (dvossel) 2009-09-01 10:51:18

Yes, this is a silly bug.  Your purposed fix would work, but we can't expand the bits on that flags variable because its already maxed out.  I'll just strip those flags out and place them in an enum on the sip_pvt.
By: pherman (pherman) 2009-09-01 11:27:16

It's maxed out on the top, but bits 3-8 seem to be free.  But yeah, if these flags are independent of each other then enum would be better.
By: David Vossel (dvossel) 2009-09-01 11:30:02

actually, this isn't a bug.  They test for the flags in a way that makes this correct.

/* testing the CALL_ONHOLD flags is this way is correct.  ONHOLD is just used as a mask for 2 bits.  Those 2 bits can be one of three values. */

if (ast_test_flag(flag, ONHOLD) == ONHOLD_INACTIVE) {
   /* blah */
}