< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

Re: [reSIProcate] Question about an assert(0) that was added in rev 8925 of ServerSubscription.cxx


You're right, it does the trick :)

Thanks,
Francis

On Thu, Sep 8, 2011 at 2:02 PM, Scott Godin <sgodin@xxxxxxxxxxxxxxx> wrote:
> Actually I think a break is missing from the previous case statement.  I'll
> commit a fix.  Let me know if you still see the issue or not.
> Thanks!
> Scott
>
> On Thu, Sep 8, 2011 at 1:33 PM, Francis Joanis <francis.joanis@xxxxxxxxx>
> wrote:
>>
>> Hi,
>>
>> I'm currently hitting an assertion when refusing a REFER's implied
>> subscription with a 503. The assertion is shown below (between
>> $$$$$$$$$$$$$):
>>
>> bool
>> ServerSubscription::shouldDestroyAfterSendingFailure(const SipMessage&
>> msg)
>> {
>>   int code = msg.header(h_StatusLine).statusCode();
>>   switch(mSubDlgState)
>>   {
>>      case SubDlgInitial:
>>         return true;
>>      case SubDlgTerminating: //terminated state not using in
>> ServerSubscription
>>         assert(0);
>>         return true;
>>      case SubDlgEstablished:
>>      {
>>         if (code == 405)
>>         {
>>            return true;
>>         }
>>         switch (Helper::determineFailureMessageEffect(*mLastResponse))
>>         {
>>            case Helper::TransactionTermination:
>>            case Helper::RetryAfter:
>>               break;
>>            case Helper::OptionalRetryAfter:
>>            case Helper::ApplicationDependant:
>>               // .bwc. Uh, no. ApplicationDependent should imply that the
>>               // app-writer has decided what to do. We don't decide here.
>> And
>>               // OptionalRetryAfter certainly doesn't mean we should tear
>> the
>>               // Usage down.
>> //               throw UsageUseException("Not a reasonable code to
>> reject a SUBSCIRBE(refresh) inside a dialog.",
>> //                                       __FILE__, __LINE__);
>>               break;
>>            case Helper::DialogTermination: //?dcm? -- throw or destroy
>> this?
>>            case Helper::UsageTermination:
>>               return true;
>>         }
>>      }
>>      default: // !jf!
>> $$$$$$$$$$$$$         assert(0); $$$$$$$$$$$$$
>>         break;
>>
>>   }
>>   return false;
>> }
>>
>> I'm no expert at this part of the code (I guess, yet ;) ), but it
>> looks like either this assert is invalid or some "return false"
>> statements are missing in the nested switch statement.
>>
>> I'm thinking the assert should be removed but I'd like to get your
>> opinions first.
>>
>> P.S. The diff can be seen here:
>>
>> https://svn.resiprocate.org/viewsvn/resiprocate/main/resip/dum/ServerSubscription.cxx?r1=8919&r2=8925
>>
>> Thanks,
>> Francis
>> _______________________________________________
>> resiprocate-devel mailing list
>> resiprocate-devel@xxxxxxxxxxxxxxx
>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>