[asterisk-commits] trunk r18792 - /trunk/channels/chan_sip.c
asterisk-commits at lists.digium.com
asterisk-commits at lists.digium.com
Mon Apr 10 04:57:43 MST 2006
Author: rizzo
Date: Mon Apr 10 06:57:40 2006
New Revision: 18792
URL: http://svn.digium.com/view/asterisk?rev=18792&view=rev
Log:
- constification of some functions (args and return values):
transmit_response_reliable()
hangup_cause2sip()
- remove useless braces;
- add comments on some slightly cryptic code segments
- mark XXX possible critical pieces of code.
- remove an unneeded string termination after ast_copy_string
- mark usage of some rarely used functions;
- use strsep() instead of emulating it inline;
- replace magic constants with sizeof(array)
Modified:
trunk/channels/chan_sip.c
Modified: trunk/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/trunk/channels/chan_sip.c?rev=18792&r1=18791&r2=18792&view=diff
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Mon Apr 10 06:57:40 2006
@@ -967,7 +967,7 @@
static void parse_request(struct sip_request *req);
static const char *get_header(const struct sip_request *req, const char *name);
static void copy_request(struct sip_request *dst,struct sip_request *src);
-static int transmit_response_reliable(struct sip_pvt *p, char *msg, struct sip_request *req);
+static int transmit_response_reliable(struct sip_pvt *p, const char *msg, struct sip_request *req);
static int transmit_register(struct sip_registry *r, int sipmethod, char *auth, char *authheader);
static int sip_poke_peer(struct sip_peer *peer);
static int __sip_do_register(struct sip_registry *r);
@@ -2440,10 +2440,9 @@
31 normal unspecified 480 Temporarily unavailable
\endverbatim
*/
-static char *hangup_cause2sip(int cause)
-{
- switch(cause)
- {
+static const char *hangup_cause2sip(int cause)
+{
+ switch (cause) {
case AST_CAUSE_UNALLOCATED: /* 1 */
case AST_CAUSE_NO_ROUTE_DESTINATION: /* 3 IAX2: Can't find extension in context */
case AST_CAUSE_NO_ROUTE_TRANSIT_NET: /* 2 */
@@ -2552,10 +2551,10 @@
update_call_counter(p, INC_CALL_LIMIT);
}
} else { /* Incoming call, not up */
- char *res;
- if (ast->hangupcause && ((res = hangup_cause2sip(ast->hangupcause)))) {
+ const char *res;
+ if (ast->hangupcause && (res = hangup_cause2sip(ast->hangupcause)))
transmit_response_reliable(p, res, &p->initreq);
- } else
+ else
transmit_response_reliable(p, "603 Declined", &p->initreq);
}
} else { /* Call is in UP state, send BYE */
@@ -3983,14 +3982,15 @@
p = r;
for (;route ; route = route->next) {
n = strlen(route->hop);
- if ( n + 3 > rem)
+ if (rem < n+3) /* we need room for ",<route>" */
break;
- if (p != r) {
+ if (p != r) { /* add a separator after fist route */
*p++ = ',';
--rem;
}
*p++ = '<';
- ast_copy_string(p, route->hop, rem); p += n;
+ ast_copy_string(p, route->hop, rem); /* cannot fail */
+ p += n;
*p++ = '>';
rem -= (n+2);
}
@@ -4029,6 +4029,7 @@
if (hn > sizeof(hostname))
hn = sizeof(hostname);
ast_copy_string(hostname, h, hn);
+ /* XXX bug here if string has been trimmed to sizeof(hostname) */
h += hn - 1;
/* Is "port" present? if not default to DEFAULT_SIP_PORT */
@@ -4115,10 +4116,8 @@
snprintf(newto, sizeof(newto), "%s;tag=%s", ot, p->theirtag);
else if (p->tag && !ast_test_flag(&p->flags[0], SIP_OUTGOING))
snprintf(newto, sizeof(newto), "%s;tag=%s", ot, p->tag);
- else {
+ else
ast_copy_string(newto, ot, sizeof(newto));
- newto[sizeof(newto) - 1] = '\0';
- }
ot = newto;
}
add_header(resp, "To", ot);
@@ -4247,7 +4246,7 @@
}
/*! \brief Base transmit response function */
-static int __transmit_response(struct sip_pvt *p, char *msg, struct sip_request *req, enum xmittype reliable)
+static int __transmit_response(struct sip_pvt *p, const char *msg, struct sip_request *req, enum xmittype reliable)
{
struct sip_request resp;
int seqno = 0;
@@ -4286,7 +4285,7 @@
/*! \brief Transmit response, Make sure you get an ACK
This is only used for responses to INVITEs, where we need to make sure we get an ACK
*/
-static int transmit_response_reliable(struct sip_pvt *p, char *msg, struct sip_request *req)
+static int transmit_response_reliable(struct sip_pvt *p, const char *msg, struct sip_request *req)
{
return __transmit_response(p, msg, req, XMIT_CRITICAL);
}
@@ -5107,10 +5106,7 @@
switch (state) {
case (AST_EXTENSION_RINGING | AST_EXTENSION_INUSE):
- if (global_notifyringing)
- statestring = "early";
- else
- statestring = "confirmed";
+ statestring = (global_notifyringing) ? "early" : "confirmed";
local_state = NOTIFY_INUSE;
pidfstate = "busy";
pidfnote = "Ringing";
@@ -5272,7 +5268,8 @@
add_header(&req, "Content-Type", default_notifymime);
ast_build_string(&t, &maxbytes, "Messages-Waiting: %s\r\n", newmsgs ? "yes" : "no");
- ast_build_string(&t, &maxbytes, "Message-Account: sip:%s@%s\r\n", !ast_strlen_zero(vmexten) ? vmexten : default_vmexten, S_OR(p->fromdomain, ast_inet_ntoa(iabuf, sizeof(iabuf), p->ourip)));
+ ast_build_string(&t, &maxbytes, "Message-Account: sip:%s@%s\r\n",
+ S_OR(vmexten, default_vmexten), S_OR(p->fromdomain, ast_inet_ntoa(iabuf, sizeof(iabuf), p->ourip)));
ast_build_string(&t, &maxbytes, "Voice-Message: %d/%d (0/0)\r\n", newmsgs, oldmsgs);
if (p->subscribed) {
if (p->expiry)
@@ -5423,7 +5420,7 @@
r->call = NULL;
ast_set_flag(&p->flags[0], SIP_NEEDDESTROY);
/* Pretend to ACK anything just in case */
- __sip_pretend_ack(p);
+ __sip_pretend_ack(p); /* XXX we need p locked, not sure we have */
}
/* If we have a limit, stop registration and give up */
if (global_regattempts_max && (r->regattempts > global_regattempts_max)) {
@@ -5654,9 +5651,9 @@
ast_copy_string(from, of, sizeof(from));
of = get_in_brackets(from);
ast_string_field_set(p, from, of);
- if (strncmp(of, "sip:", 4)) {
+ if (strncmp(of, "sip:", 4))
ast_log(LOG_NOTICE, "From address missing 'sip:', using it anyway\n");
- } else
+ else
of += 4;
/* Get just the username part */
if ((c = strchr(dest, '@')))
@@ -5899,6 +5896,7 @@
/* Work on a copy */
contact = ast_strdupa(pvt->fullcontact);
+ /* XXX this code is repeated all over */
/* Make sure it's a SIP URL */
if (strncasecmp(contact, "sip:", 4)) {
ast_log(LOG_NOTICE, "'%s' is not a valid SIP contact (missing sip:) trying to use anyway\n", contact);
@@ -6088,9 +6086,8 @@
useragent = get_header(req, "User-Agent");
if (useragent && strcasecmp(useragent, p->useragent)) {
ast_copy_string(p->useragent, useragent, sizeof(p->useragent));
- if (option_verbose > 3) {
+ if (option_verbose > 3)
ast_verbose(VERBOSE_PREFIX_3 "Saved useragent \"%s\" for peer %s\n",p->useragent,p->name);
- }
}
return PARSE_REGISTER_UPDATE;
}
@@ -6367,11 +6364,12 @@
/* Schedule auto destroy in 32 seconds */
sip_scheddestroy(p, 32000);
- return 1; /* Challenge sent */
+ return 1; /* XXX should it be -1 ? */
}
if (good_response) /* Auth is OK */
return 0;
+ /* XXX is this needed ? */
/* Ok, we have a bad username/secret pair */
/* Challenge again, and again, and again */
transmit_response_with_auth(p, response, req, p->randdata, reliable, respheader, 0);
@@ -6573,9 +6571,8 @@
return -1;
}
c += 4;
- if ((a = strchr(c, '@')) || (a = strchr(c, ';'))) {
- *a = '\0';
- }
+ a = c;
+ strsep(&a, "@;"); /* trim anything after @ or ; */
if (sip_debug_test_pvt(p))
ast_verbose("RDNIS is %s\n", c);
ast_string_field_set(p, rdnis, c);
@@ -6983,7 +6980,7 @@
* Returns true if number should be restricted (privacy setting found)
* output is set to NULL if no number found
*/
-static int get_rpid_num(const char *input,char *output, int maxlen)
+static int get_rpid_num(const char *input, char *output, int maxlen)
{
char *start;
char *end;
@@ -7041,6 +7038,7 @@
ast_copy_string(from, get_header(req, "From"), sizeof(from)); /* XXX bug in original code, overwrote string */
if (pedanticsipchecking)
ast_uri_decode(from);
+ /* XXX here tries to map the username for invite things */
memset(calleridname, 0, sizeof(calleridname));
get_calleridname(from, calleridname, sizeof(calleridname));
if (calleridname[0])
@@ -7272,6 +7270,7 @@
if (!ast_strlen_zero(peer->username)) {
ast_string_field_set(p, username, peer->username);
/* Use the default username for authentication on outbound calls */
+ /* XXX this takes the name from the caller... can we override ? */
ast_string_field_set(p, authname, peer->username);
}
if (!ast_strlen_zero(peer->cid_num) && !ast_strlen_zero(p->cid_num)) {
@@ -7553,7 +7552,7 @@
int total = 0;
if (!ast_strlen_zero(id))
- snprintf(idtext,256,"ActionID: %s\r\n",id);
+ snprintf(idtext, sizeof(idtext), "ActionID: %s\r\n", id);
astman_send_ack(s, m, "Peer status list will follow");
/* List the peers in separate manager events */
@@ -7596,7 +7595,7 @@
if (s) { /* Manager - get ActionID */
id = astman_get_header(m,"ActionID");
if (!ast_strlen_zero(id))
- snprintf(idtext,256,"ActionID: %s\r\n",id);
+ snprintf(idtext, sizeof(idtext), "ActionID: %s\r\n", id);
}
switch (argc) {
@@ -9926,6 +9925,7 @@
}
/*! \brief Handle SIP response in dialogue */
+/* XXX only called by handle_request */
static void handle_response(struct sip_pvt *p, int resp, char *rest, struct sip_request *req, int ignore, int seqno)
{
struct ast_channel *owner;
@@ -11284,6 +11284,7 @@
/*! \brief Handle incoming SIP requests (methods)
\note This is where all incoming requests go first */
+/* called with p and p->owner locked */
static int handle_request(struct sip_pvt *p, struct sip_request *req, struct sockaddr_in *sin, int *recount, int *nounlock)
{
/* Called with p->lock held, as well as p->owner->lock if appropriate, keeping things
@@ -11540,9 +11541,10 @@
/* Process request, with netlock held */
retrylock:
ast_mutex_lock(&netlock);
- p = find_call(&req, &sin, req.method);
+ p = find_call(&req, &sin, req.method); /* returns p locked */
if (p) {
/* Go ahead and lock the owner if it has one -- we may need it */
+ /* becaues this is deadlock-prone, we need to try and unlock if failed */
if (p->owner && ast_mutex_trylock(&p->owner->lock)) {
ast_log(LOG_DEBUG, "Failed to grab lock, trying again...\n");
ast_mutex_unlock(&p->lock);
@@ -12291,7 +12293,7 @@
if (!strcasecmp(a->realm, realm))
break;
}
-
+
return a;
}
@@ -12502,6 +12504,7 @@
if (peer->chanvars) {
ast_variables_destroy(peer->chanvars);
peer->chanvars = NULL;
+ /* XXX should unregister ? */
}
for (; v; v = v->next) {
if (handle_common_options(&peerflags[0], &mask[0], v))
@@ -12642,8 +12645,7 @@
/* Set peer channel variable */
varname = ast_strdupa(v->value);
if (varname && (varval = strchr(varname,'='))) {
- *varval = '\0';
- varval++;
+ *varval++ = '\0';
if ((tmpvar = ast_variable_new(varname, varval))) {
tmpvar->next = peer->chanvars;
peer->chanvars = tmpvar;
@@ -13207,8 +13209,7 @@
}
}
/* Reset lastrtprx timer */
- time(&p->lastrtprx);
- time(&p->lastrtptx);
+ p->lastrtprx = p->lastrtptx = time(NULL);
ast_mutex_unlock(&p->lock);
return 0;
}
More information about the asterisk-commits
mailing list