< 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


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