networking: more assertions and cleanups to size_t/unsigned changes

Basically, I don't trust myself nor Max to not have bugs in our
code when switching over to unsigned types, so I've added more
assertions which will hopefully trip and force us to fix these
bugs before somebody can exploit them :)

Some cleanups for parameter parsing using strtol
and error reporting to the user.  Also, fix some completely
garbled indentation in inputStream_http.c

git-svn-id: https://svn.musicpd.org/mpd/trunk@7209 09075e82-0dd4-0310-85a5-a0d7c8717e4f

2 files changed, 58 insertions, 48 deletions

diff --git a/src/inputStream_http.c b/src/inputStream_http.c
index 9e9bbc921..23434beb6 100644
--- a/src/inputStream_http.c
+++ b/src/inputStream_http.c
@@ -57,8 +57,8 @@ typedef struct _InputStreemHTTPData {
 	size_t icyOffset;
 	char *proxyAuth;
 	char *httpAuth;
-    /* Number of times mpd tried to get data */
-    int tries;
+	/* Number of times mpd tried to get data */
+	int tries;
 } InputStreamHTTPData;
 
 void inputStream_initHttp(void)
@@ -113,37 +113,34 @@ void inputStream_initHttp(void)
 	param = getConfigParam(CONF_HTTP_BUFFER_SIZE);
 
 	if (param) {
-		bufferSize = strtoul(param->value, &test, 10);
-
-		if (*test != '\0') {
+		long tmp = strtol(param->value, &test, 10);
+		if (*test != '\0' || tmp <= 0) {
 			FATAL("\"%s\" specified for %s at line %i is not a "
 			      "positive integer\n",
 			      param->value, CONF_HTTP_BUFFER_SIZE, param->line);
 		}
 
-		bufferSize *= 1024;
-
-		if (prebufferSize > bufferSize)
-			prebufferSize = bufferSize;
+		bufferSize = tmp * 1024;
 	}
 
 	param = getConfigParam(CONF_HTTP_PREBUFFER_SIZE);
 
 	if (param) {
-		prebufferSize = strtoul(param->value, &test, 10);
-
-		if (prebufferSize <= 0 || *test != '\0') {
+		long tmp = strtol(param->value, &test, 10);
+		if (*test != '\0' || tmp <= 0) {
 			FATAL("\"%s\" specified for %s at line %i is not a "
 			      "positive integer\n",
 			      param->value, CONF_HTTP_PREBUFFER_SIZE,
 			      param->line);
 		}
 
-		prebufferSize *= 1024;
+		prebufferSize = tmp * 1024;
 	}
 
 	if (prebufferSize > bufferSize)
 		prebufferSize = bufferSize;
+	assert(bufferSize > 0 && "http bufferSize too small");
+	assert(prebufferSize > 0 && "http prebufferSize too small");
 }
 
 /* base64 code taken from xmms */
@@ -157,7 +154,7 @@ static char *base64Dup(char *s)
 	char *ret = xcalloc(BASE64_LENGTH(len) + 1, 1);
 	unsigned char *p = (unsigned char *)ret;
 
-	char tbl[64] = {
+	static const char tbl[64] = {
 		'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H',
 		'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P',
 		'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X',
@@ -240,7 +237,7 @@ static InputStreamHTTPData *newInputStreamHTTPData(void)
 	ret->prebuffer = 0;
 	ret->icyOffset = 0;
 	ret->buffer = xmalloc(bufferSize);
-    ret->tries = 0;
+	ret->tries = 0;
 	return ret;
 }
 
@@ -776,6 +773,9 @@ size_t inputStream_httpRead(InputStream * inStream, void *ptr, size_t size,
 	if (data->icyMetaint > 0) {
 		if (data->icyOffset >= data->icyMetaint) {
 			size_t metalen = *(data->buffer);
+			/* maybe we're in some strange universe where a byte
+			 * can hold more than 255 ... */
+			assert(metalen <= 255 && "metalen greater than 255");
 			metalen <<= 4;
 			if (metalen + 1 > data->buflen) {
 				/* damn that's some fucking big metadata! */
@@ -796,6 +796,8 @@ size_t inputStream_httpRead(InputStream * inStream, void *ptr, size_t size,
 				data->buflen);
 			data->icyOffset = 0;
 		}
+		assert(data->icyOffset <= data->icyMetaint &&
+		       "icyOffset bigger than icyMetaint!");
 		maxToSend = data->icyMetaint - data->icyOffset;
 		maxToSend = maxToSend > data->buflen ? data->buflen : maxToSend;
 	}
@@ -879,25 +881,25 @@ int inputStream_httpBuffer(InputStream * inStream)
 	    data->buflen < bufferSize - 1) {
 		readed = read(data->sock, data->buffer + data->buflen,
 			      bufferSize - 1 - data->buflen);
-        /* If the connection is currently unavailable, or interrupted (EINTR)
-         * Don't give an error, so it's retried later.
-         * Max times in a row to re-try this is HTTP_MAX_TRIES
-         */
-		if (readed < 0 && (errno == EAGAIN || errno == EINTR) && data->tries < HTTP_MAX_TRIES) {
-            data->tries++;
-            DEBUG(__FILE__": Resource unavailable, trying %i times again\n", HTTP_MAX_TRIES-data->tries);
-            readed = 0;
+		/*
+		 * If the connection is currently unavailable, or
+		 * interrupted (EINTR)
+		 * Don't give an error, so it's retried later.
+		 * Max times in a row to retry this is HTTP_MAX_TRIES
+		 */
+		if (readed < 0 &&
+		    (errno == EAGAIN || errno == EINTR) &&
+		    data->tries < HTTP_MAX_TRIES) {
+			data->tries++;
+			DEBUG(__FILE__": Resource unavailable, trying %i "
+			      "times again\n", HTTP_MAX_TRIES - data->tries);
+			readed = 0;
 		} else if (readed <= 0) {
-			close(data->sock);
-            if(errno)
-                DEBUG(__FILE__": Closing connection with error: '%s'\n", strerror(errno));
+			while (close(data->sock) && errno == EINTR);
 			data->connState = HTTP_CONN_STATE_CLOSED;
 			readed = 0;
-		}
-        else{
-            /* Reset the tries back to 0, because we managed to read data */
-            data->tries = 0;
-        }
+		} else /* readed > 0, reset */
+			data->tries = 0;
 
 		data->buflen += readed;
 	}
diff --git a/src/interface.c b/src/interface.c
index 029025cbf..0beffba23 100644
--- a/src/interface.c
+++ b/src/interface.c
@@ -318,7 +318,8 @@ static int processLineOfInput(Interface * interface)
 				      "(%lu)\n",
 				      interface->num,
 				      (unsigned long)interface->cmd_list_size,
-				      (unsigned long)interface_max_command_list_size);
+				      (unsigned long)
+				      interface_max_command_list_size);
 				closeInterface(interface);
 				ret = COMMAND_RETURN_CLOSE;
 			} else
@@ -362,7 +363,7 @@ static int processBytesRead(Interface * interface, int bytesRead)
 		buf_tail++;
 		if (*buf_tail == '\n') {
 			*buf_tail = '\0';
-			if (interface->bufferLength - interface->bufferPos > 1) {
+			if (interface->bufferLength > interface->bufferPos) {
 				if (*(buf_tail - 1) == '\r')
 					*(buf_tail - 1) = '\0';
 			}
@@ -382,6 +383,8 @@ static int processBytesRead(Interface * interface, int bytesRead)
 			    interface->cmd_list &&
 			    !interface->cmd_list_dup)
 				cmd_list_clone(interface);
+			assert(interface->bufferLength >= interface->bufferPos
+			       && "bufferLength >= bufferPos");
 			interface->bufferLength -= interface->bufferPos;
 			memmove(interface->buffer,
 				interface->buffer + interface->bufferPos,
@@ -563,25 +566,23 @@ void initInterfaces(void)
 	param = getConfigParam(CONF_MAX_COMMAND_LIST_SIZE);
 
 	if (param) {
-		interface_max_command_list_size = strtol(param->value,
-							 &test, 10);
-		if (*test != '\0' || interface_max_command_list_size <= 0) {
+		long tmp = strtol(param->value, &test, 10);
+		if (*test != '\0' || tmp <= 0) {
 			FATAL("max command list size \"%s\" is not a positive "
 			      "integer, line %i\n", param->value, param->line);
 		}
-		interface_max_command_list_size *= 1024;
+		interface_max_command_list_size = tmp * 1024;
 	}
 
 	param = getConfigParam(CONF_MAX_OUTPUT_BUFFER_SIZE);
 
 	if (param) {
-		interface_max_output_buffer_size = strtol(param->value,
-		                                          &test, 10);
-		if (*test != '\0' || interface_max_output_buffer_size <= 0) {
+		long tmp = strtol(param->value, &test, 10);
+		if (*test != '\0' || tmp <= 0) {
 			FATAL("max output buffer size \"%s\" is not a positive "
 			      "integer, line %i\n", param->value, param->line);
 		}
-		interface_max_output_buffer_size *= 1024;
+		interface_max_output_buffer_size = tmp * 1024;
 	}
 
 	interfaces = xmalloc(sizeof(Interface) * interface_max_connections);
@@ -650,13 +651,16 @@ static void flushInterfaceBuffer(Interface * interface)
 		if (ret < 0)
 			break;
 		else if ((size_t)ret < buf->size) {
+			assert(interface->deferred_bytes >= ret);
 			interface->deferred_bytes -= ret;
 			buf->data = (char *)buf->data + ret;
 			buf->size -= ret;
 		} else {
 			struct sllnode *tmp = buf;
-			interface->deferred_bytes -= (buf->size +
-						      sizeof(struct sllnode));
+			size_t decr = (buf->size + sizeof(struct sllnode));
+
+			assert(interface->deferred_bytes >= decr);
+			interface->deferred_bytes -= decr;
 			buf = buf->next;
 			free(tmp);
 			interface->deferred_send = buf;
@@ -709,7 +713,11 @@ int interfacePrintWithFD(int fd, char *buffer, size_t buflen)
 	interface = interfaces + i;
 
 	while (buflen > 0 && !interface->expired) {
-		size_t left = interface->send_buf_size - interface->send_buf_used;
+		size_t left;
+
+		assert(interface->send_buf_size >= interface->send_buf_used);
+		left = interface->send_buf_size - interface->send_buf_used;
+
 		copylen = buflen > left ? left : buflen;
 		memcpy(interface->send_buf + interface->send_buf_used, buffer,
 		       copylen);
@@ -737,11 +745,11 @@ static void printInterfaceOutBuffer(Interface * interface)
 		                             + interface->send_buf_used;
 		if (interface->deferred_bytes >
 		    interface_max_output_buffer_size) {
-			ERROR("interface %i: output buffer size (%li) is "
-			      "larger than the max (%li)\n",
+			ERROR("interface %i: output buffer size (%lu) is "
+			      "larger than the max (%lu)\n",
 			      interface->num,
-			      (long)interface->deferred_bytes,
-			      (long)interface_max_output_buffer_size);
+			      (unsigned long)interface->deferred_bytes,
+			      (unsigned long)interface_max_output_buffer_size);
 			/* cause interface to close */
 			interface->expired = 1;
 			do {