[asterisk-commits] wedhorn: branch wedhorn/skinny-session r384238 - /team/wedhorn/skinny-session...

[SVN commits to the Asterisk project]

Author: wedhorn
Date: Thu Mar 28 03:27:13 2013
New Revision: 384238

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=384238
Log:
skinny: cleanup session error logging and move unlocking on error to end

Modified:
    team/wedhorn/skinny-session/channels/chan_skinny.c

Modified: team/wedhorn/skinny-session/channels/chan_skinny.c
URL: http://svnview.digium.com/svn/asterisk/team/wedhorn/skinny-session/channels/chan_skinny.c?view=diff&rev=384238&r1=384237&r2=384238
==============================================================================
--- team/wedhorn/skinny-session/channels/chan_skinny.c (original)
+++ team/wedhorn/skinny-session/channels/chan_skinny.c Thu Mar 28 03:27:13 2013
@@ -7429,6 +7429,7 @@
 	int timeout;
 	time_t now;
 	struct pollfd fds[1];
+	int lockstate = 0;
 
 	if (!s) {
 		ast_verb(3, "Bad Skinny Session\n");
@@ -7486,55 +7487,43 @@
 			}
 
 			ast_mutex_lock(&s->lock);
-			res = read(s->fd, req, skinny_header_size);
-			if (res < 0) {
-				ast_log(LOG_WARNING, "read() returned error: %s\n", strerror(errno));
-
-				ast_log(LOG_WARNING, "Skinny Client was lost, unregistering\n");
-
-				ast_mutex_unlock(&s->lock);
-				ast_verb(3, "Ending Skinny session from %s (bad input)\n", ast_inet_ntoa(s->sin.sin_addr));
-				break;
-			} else if (res != skinny_header_size) {
-				ast_log(LOG_WARNING, "Skinny Client sent less data than expected.  Expected 4 but got %d.\n", res);
-				ast_mutex_unlock(&s->lock);
-
-				if (res == 0) {
-					ast_log(LOG_WARNING, "Skinny Client was lost, unregistering\n");
+			lockstate = 1;
+
+			if ((res = read(s->fd, req, skinny_header_size)) != skinny_header_size) {
+				if (res < 0) {
+					ast_log(LOG_WARNING, "Header read() returned error: %s\n", strerror(errno));
+				} else {
+					ast_log(LOG_WARNING, "Unable to read header. Only found %d bytes.\n", res);
 				}
-				ast_verb(3, "Ending Skinny session from %s (bad input)\n", ast_inet_ntoa(s->sin.sin_addr));
-				break;
-			}
-
-			dlen = letohl(req->len);
-			if (dlen < 4) {
-				ast_log(LOG_WARNING, "Skinny Client sent invalid data.\n");
-				ast_mutex_unlock(&s->lock);
-				ast_verb(3, "Ending Skinny session from %s (bad input)\n", ast_inet_ntoa(s->sin.sin_addr));
-				break;
-			}
-			if (dlen+8 > SKINNY_MAX_PACKET) {
-				ast_log(LOG_WARNING, "Skinny packet too large (%d bytes), max length(%d bytes)\n", dlen+8, SKINNY_MAX_PACKET);
-				dlen = sizeof(req) - 8;
-			}
-
-			res = read(s->fd, &req->data, dlen-4);
-			ast_mutex_unlock(&s->lock);
-			if (res < 0) {
-				ast_log(LOG_WARNING, "read() returned error: %s\n", strerror(errno));
-				ast_verb(3, "Ending Skinny session from %s (bad input)\n", ast_inet_ntoa(s->sin.sin_addr));
-				break;
-			} else if (res != (dlen-4)) {
-				ast_log(LOG_WARNING, "Skinny Client sent less data than expected.\n");
-				ast_verb(3, "Ending Skinny session from %s (bad input)\n", ast_inet_ntoa(s->sin.sin_addr));
 				break;
 			}
 
 			if (letohl(req->e) < 0) {
 				ast_log(LOG_ERROR, "Event Message is NULL from socket %d, This is bad\n", s->fd);
-				ast_verb(3, "Ending Skinny session from %s (failed parse)\n", ast_inet_ntoa(s->sin.sin_addr));
 				break;
 			}
+
+			dlen = letohl(req->len) - 4;
+			if (dlen < 0) {
+				ast_log(LOG_WARNING, "Skinny Client sent invalid data.\n");
+				break;
+			}
+			if (dlen > (SKINNY_MAX_PACKET - skinny_header_size)) {
+				ast_log(LOG_WARNING, "Skinny packet too large (%d bytes), max length(%d bytes)\n", dlen+8, SKINNY_MAX_PACKET);
+				break;
+			}
+
+			if ((res = read(s->fd, &req->data, dlen)) != dlen) {
+				if (res < 0) {
+					ast_log(LOG_WARNING, "Data read() returned error: %s\n", strerror(errno));
+				} else {
+					ast_log(LOG_WARNING, "Unable to read data. Only found %d bytes.\n", res);
+				}
+				break;
+			}
+			
+			ast_mutex_unlock(&s->lock);
+			lockstate = 0;
 
 			res = handle_message(req, s);
 			if (res < 0) {
@@ -7550,6 +7539,9 @@
 	}
 
 	ast_debug(3, "Skinny Session returned: %s\n", strerror(errno));
+	if (lockstate) {
+		ast_mutex_unlock(&s->lock);
+	}
 	if (req) {
 		ast_free(req);
 	}