From: Boulkroune, O. (Non-HP:A. Origin) <oli...@hp...> - 2007-09-12 15:47:58
|
Charles, You did what I chose not to do :) .=20 The reason of my commit is a bug which occurs when: - we aborted a call sending an ACK, then a BYE : the call is then = deleted=20 - we then received a BYE from the remote part for this call: the call = id is not found (for it was removed from the map). However, we must = answer it with a 200 OK, otherwise it would be repeated by the remote - the -aa option is set, so we created a new call just for sending a 200 = OK response. But doing this, we incremented the call-id counter as well = as the input files line numbers=20 - therefore, the next new incoming call will skip a line in the input = files.... So, we are here in a very specific case, which should happen very rarely = (I just met this situation with HP testers for the first time).=20 For performance reasons, I wanted to avoid to put a conditional = statement in the call class constructor, as well as an additional = function in the add_call function, i.e impact every call creation = process, just for handling this particular case.=20 I looked without success for other solutions, such as sending this 200 = OK message outside any call, which is impossible, or making the aborting = calls wait for its 200 OK response before exiting, which could bring = many other problems. Things would have been much easier if in this particular case we needed = to have an additional operation to do; I would have created a new = automatic_call class inheriting from the call class and overloaded the = new class constructor just to add this operation. But we are here in the = opposite situation, i.e an operation needs to be removed. The cleanest way to get out of this would have been to create a new = class and make the call class inherit from it. But this would have = needed a lot of structural changes, making this potentially = dangerous...and would have been longer to write. I thought the handling = of this particular case was not worth doing this. That's why I prefered to copy/paste a large part of code, which is = pretty ugly and might also be dangerous, instead of impacting the tool = performances. Note that this copy/paste could perhaps be cleaned, for we = might don't need to initialize every member of the call class in our = overloaded constructor, the using of this call object being limited. What's your meaning about this ? Olivier Boulkroune =20 -----Message d'origine----- De=A0: Charles P Wright [mailto:cpw...@us...]=20 Envoy=E9=A0: mercredi 12 septembre 2007 16:56 =C0=A0: obo...@us... Cc=A0: sip...@li... Objet=A0: Re: [Sipp-commits] SF.net SVN: sipp: [303] sipp/trunk Olivier, This seems to do a lot of copy and paste, which I think has the = potential=20 for future bugs because it is so easy to change things in one place but=20 not another. I have committed a slight revision (304) that integrates = your=20 changes into the core call::call and add_call functions with small=20 compatibility wrappers. This should make maintenance simpler in the=20 future. Charles obo...@us...=20 Sent by: sip...@li... 09/12/2007 10:10 AM To sip...@li... cc Subject [Sipp-commits] SF.net SVN: sipp: [303] sipp/trunk Revision: 303 http://sipp.svn.sourceforge.net/sipp/?rev=3D303&view=3Drev Author: oboulkroune Date: 2007-09-12 07:10:27 -0700 (Wed, 12 Sep 2007) Log Message: ----------- Fix: when we create a call to answer to an out of call message in=20 automatic response mode, we must not increment neither the call-id = counter=20 nor the input files line numbers Modified Paths: -------------- sipp/trunk/call.cpp sipp/trunk/call.hpp sipp/trunk/sipp.cpp sipp/trunk/sipp.hpp Modified: sipp/trunk/call.cpp =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sipp/trunk/call.cpp 2007-09-12 09:48:01 UTC (rev 302) +++ sipp/trunk/call.cpp 2007-09-12 14:10:27 UTC (rev 303) @@ -172,7 +172,54 @@ return new_call; } =20 +call * add_call(char * call_id , struct sipp_socket *socket, bool=20 isAutomatic) { + call * new_call; + unsigned int nb;=20 +=20 + if(!next_number) { next_number ++; } =20 + if (use_tdmmap) { + nb =3D get_tdm_map_number(next_number); + if (nb !=3D 0) { + /* Mark the entry in the list as busy */ + tdm_map[nb - 1] =3D true; + } else { + /* Can't create the new call */ + WARNING("Can't create new outgoing call: all tdm_map circuits=20 busy"); + return NULL; + } + } + + new_call =3D new call(call_id, 0, socket->ss_ipv6, true); + + if(!new_call) { + ERROR("Memory Overflow"); + } + /* All calls must exist in the map. */ + calls[std::string(call_id)] =3D new_call; + /* All calls start off in the running state. */ + add_running_call(new_call); + + new_call -> number =3D next_number; + new_call -> tdm_map_number =3D nb - 1; + + /* Vital counters update */ + /* We do not update the call_id counter, for we create here a call */ + /* to answer to an out of call message */ + open_calls++; + + /* Statistics update */ + calls_since_last_rate_change++; + total_calls ++; + + if(open_calls > open_calls_peak) { + open_calls_peak =3D open_calls; + open_calls_peak_time =3D clock_tick / 1000; + } + new_call->associate_socket(socket); + return new_call; +} + call * add_call(int userId, bool ipv6) { static char call_id[MAX_HEADER_LEN]; @@ -704,6 +751,79 @@ send_timeout =3D 0; } =20 +/* We overload the constructor to create calls to send responses to out = of call*/ +/* messages in the automatic response mode. The only difference is that = we don't increment */ +/* the output files line numbers */ + +call::call(char * p_id, int userId, bool ipv6, bool isAutomatic) :=20 use_ipv6(ipv6) +{ + memset(this, 0, sizeof(call)); + id =3D strdup(p_id); + start_time =3D clock_tick; + call_established=3Dfalse ; + count_in_stats=3Dtrue ; + ack_is_pending=3Dfalse ; + last_recv_msg =3D NULL; + cseq =3D base_cseq; + nb_last_delay =3D 0; + tdm_map_number =3D 0; + +#ifdef _USE_OPENSSL + m_ctx_ssl =3D NULL ; + m_bio =3D NULL ; +#endif + + call_remote_socket =3D 0; + + // initialising the CallVariable with the Scenario variable + int i; + if (maxVariableUsed >=3D 0) { + M_callVariableTable =3D new CCallVariable *[maxVariableUsed + 1]; + } + for(i=3D0; i<=3DmaxVariableUsed; i++) + { + if (variableUsed[i]) { + M_callVariableTable[i] =3D new CCallVariable(); + if (M_callVariableTable[i] =3D=3D NULL) { + ERROR ("call variable allocation failed"); + } + } else { + M_callVariableTable[i] =3D NULL; + } + } + + // If not updated by a message we use the start time + // information to compute rtd information + for (i =3D 0; i < MAX_RTD_INFO_LENGTH; i++) { + start_time_rtd[i] =3D getmicroseconds(); + rtd_done[i] =3D false; + } + + // by default, last action result is NO_ERROR + last_action_result =3D call::E_AR_NO_ERROR; + + this->userId =3D userId; + +/* We create this call to send an unique 200 OK response */ +/* answering to an out of call request. We must not */ +/* increment the input files line numbers to not disturb */ +/* the input files read mechanism (otherwise some lines risk */ +/* to be systematically skipped */ + +#ifdef PCAPPLAY + memset(&(play_args_a.to), 0, sizeof(struct sockaddr_storage)); + memset(&(play_args_v.to), 0, sizeof(struct sockaddr_storage)); + memset(&(play_args_a.from), 0, sizeof(struct sockaddr_storage)); + memset(&(play_args_v.from), 0, sizeof(struct sockaddr_storage)); + hasMediaInformation =3D 0; + media_thread =3D 0; +#endif + + peer_tag =3D NULL; + recv_timeout =3D 0; + send_timeout =3D 0; +} + call::~call() { deleted +=3D 1; @@ -3521,6 +3641,41 @@ return true; break; =20 + case 5: // response for an out of call message + old_last_recv_msg =3D NULL; + if (last_recv_msg !=3D NULL) { + last_recv_msg_saved =3D true; + old_last_recv_msg =3D (char *) malloc(strlen(last_recv_msg)+1); + strcpy(old_last_recv_msg,last_recv_msg); + } + // usage of last_ keywords + last_recv_msg =3D (char *) realloc(last_recv_msg, strlen(P_recv) + = 1); + strcpy(last_recv_msg, P_recv); + + WARNING("Automatic response mode for an out of call message"); + sendBuffer(createSendingMessage( + (char*)"SIP/2.0 200 OK\n" + "[last_Via:]\n" + "[last_Call-ID:]\n" + "[last_To:]\n" + "[last_From:]\n" + "[last_CSeq:]\n" + "Contact: sip:sipp@[local_ip]:[local_port]\n" + "Content-Length: 0\n\n" + , -1)) ; + + // restore previous last msg + if (last_recv_msg_saved =3D=3D true) { + last_recv_msg =3D (char *) realloc(last_recv_msg,=20 strlen(old_last_recv_msg) + 1); + strcpy(last_recv_msg, old_last_recv_msg); + if (old_last_recv_msg !=3D NULL) { + free(old_last_recv_msg); + old_last_recv_msg =3D NULL; + } + } + CStat::instance()->computeStat(CStat::E_AUTO_ANSWERED); + return true; + default: ERROR_P1("Internal error for automaticResponseMode - mode %d is not = implemented!", P_case); break ; Modified: sipp/trunk/call.hpp =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sipp/trunk/call.hpp 2007-09-12 09:48:01 UTC (rev 302) +++ sipp/trunk/call.hpp 2007-09-12 14:10:27 UTC (rev 303) @@ -199,7 +199,8 @@ /* call to continue and mark it as failed */ T_ActionResult last_action_result; =20 - call(char * id, int userId, bool ipv6 =3D false); + call(char * id, int userId, bool ipv6); + call (char *id, int userId, bool ipv6 , bool isAutomatic); ~call(); =20 /* rc =3D=3D true means call not deleted by processing */ @@ -310,6 +311,7 @@ call * add_call(int userId, bool ipv6); call * add_call(char * call_id , bool ipv6, int userId); call * add_call(char * call_id , struct sipp_socket *socket); +call * add_call(char * call_id , struct sipp_socket *socket, bool=20 isAutomatic); =20 call * get_call(char *); void delete_call(char *); Modified: sipp/trunk/sipp.cpp =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sipp/trunk/sipp.cpp 2007-09-12 09:48:01 UTC (rev 302) +++ sipp/trunk/sipp.cpp 2007-09-12 14:10:27 UTC (rev 303) @@ -2539,20 +2539,25 @@ else // mode !=3D from SERVER and 3PCC Controller B { // This is a message that is not relating to any known call - if (auto_answer =3D=3D true) { + if (auto_answer =3D=3D true) { // If auto answer mode, try to answer the incoming=20 message // with automaticResponseMode // call is discarded before exiting the block - call_ptr =3D add_call(call_id, socket); - if (call_ptr) { - socket->ss_count++; - call_ptr->last_recv_msg =3D (char *)=20 realloc(call_ptr->last_recv_msg, strlen(msg) + 1); - strcpy(call_ptr->last_recv_msg, msg); - call_ptr->automaticResponseMode(4, msg); - delete_call(call_id); - call_ptr =3D NULL; - total_calls--; - } + if(!get_reply_code(msg)){=20 + call_ptr =3D add_call(call_id, socket, true); + if (call_ptr) { + socket->ss_count++; + call_ptr->last_recv_msg =3D (char *)=20 realloc(call_ptr->last_recv_msg, strlen(msg) + 1); + strcpy(call_ptr->last_recv_msg, msg); + call_ptr->automaticResponseMode(5, msg); + delete_call(call_id); + call_ptr =3D NULL; + total_calls--; + } + } else{ + /* We received a response not relating to any known call */ + /* Do nothing, even if in auto answer mode */ + } } else { nb_out_of_the_blue++; CStat::instance()->computeStat Modified: sipp/trunk/sipp.hpp =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sipp/trunk/sipp.hpp 2007-09-12 09:48:01 UTC (rev 302) +++ sipp/trunk/sipp.hpp 2007-09-12 14:10:27 UTC (rev 303) @@ -93,7 +93,7 @@ =20 /************************** Constants **************************/ =20 -#define SIPP_VERSION 20070906 +#define SIPP_VERSION 20070912 #define T_UDP 0 #define T_TCP 1 #define T_TLS 2 This was sent by the SourceForge.net collaborative development platform, = the world's largest Open Source development site. -------------------------------------------------------------------------= This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Sipp-commits mailing list Sip...@li... https://lists.sourceforge.net/lists/listinfo/sipp-commits |