WT-2651: Coverity 1355591 resource leak (#2737)

* Free memory when returning an error.
* Remove unnecessary parenthesis.
* Make repeated addition/subtraction operator order of evaluation explicit.
* Remove repeated #includes.
* Some minor restructuring of the config file parsing to handle more
complicated line continuation and whitespace combinations.

2 files changed, 84 insertions, 89 deletions

diff --git a/bench/wtperf/config.c b/bench/wtperf/config.c
index e2f541b2819..7e3acab35c7 100644
--- a/bench/wtperf/config.c
+++ b/bench/wtperf/config.c
@@ -489,16 +489,18 @@ config_opt(CONFIG *cfg, WT_CONFIG_ITEM *k, WT_CONFIG_ITEM *v)
 		if (*strp == NULL)
 			begin = newstr = dstrdup(v->str);
 		else {
-			newlen += (strlen(*strp) + 1);
+			newlen += strlen(*strp) + 1;
 			newstr = dcalloc(newlen, sizeof(char));
 			snprintf(newstr, newlen,
 			    "%s,%*s", *strp, (int)v->len, v->str);
 			/* Free the old value now we've copied it. */
 			free(*strp);
-			begin = &newstr[newlen - 1 - v->len];
+			begin = &newstr[(newlen - 1) - v->len];
 		}
-		if ((ret = config_unescape(begin)) != 0)
+		if ((ret = config_unescape(begin)) != 0) {
+			free(newstr);
 			return (ret);
+		}
 		*strp = newstr;
 		break;
 	case STRING_TYPE:
@@ -538,84 +540,99 @@ config_opt(CONFIG *cfg, WT_CONFIG_ITEM *k, WT_CONFIG_ITEM *v)
 int
 config_opt_file(CONFIG *cfg, const char *filename)
 {
-	struct stat sb;
-	ssize_t read_size;
-	size_t buf_size, linelen, optionpos;
-	int contline, fd, linenum, ret;
-	char option[1024];
-	char *comment, *file_buf, *line, *ltrim, *rtrim;
+	FILE *fp;
+	size_t linelen, optionpos;
+	int linenum, ret;
+	bool contline;
+	char line[4 * 1024], option[4 * 1024];
+	char *comment, *ltrim, *rtrim;
 
-	file_buf = NULL;
+	ret = 0;
 
-	if ((fd = open(filename, O_RDONLY)) == -1) {
+	if ((fp = fopen(filename, "r")) == NULL) {
 		fprintf(stderr, "wtperf: %s: %s\n", filename, strerror(errno));
 		return (errno);
 	}
-	if ((ret = fstat(fd, &sb)) != 0) {
-		fprintf(stderr, "wtperf: stat of %s: %s\n",
-		    filename, strerror(errno));
-		ret = errno;
-		goto err;
-	}
-	buf_size = (size_t)sb.st_size;
-	file_buf = dcalloc(buf_size + 2, 1);
-	read_size = read(fd, file_buf, buf_size);
-	if (read_size == -1
-#ifndef _WIN32
-	/* Windows automatically translates \r\n -> \n so counts will be off */
-	|| (size_t)read_size != buf_size
-#endif
-	) {
-		fprintf(stderr,
-		    "wtperf: read unexpected amount from config file\n");
-		ret = EINVAL;
-		goto err;
-	}
-	/* Make sure the buffer is terminated correctly. */
-	file_buf[read_size] = '\0';
 
-	ret = 0;
 	optionpos = 0;
 	linenum = 0;
-	/*
-	 * We should switch this from using strtok to generating a single
-	 * WiredTiger configuration string compatible string, and using
-	 * the WiredTiger configuration parser to parse it at once.
-	 */
-#define	WTPERF_CONFIG_DELIMS	"\n"
-	for (line = strtok(file_buf, WTPERF_CONFIG_DELIMS);
-	    line != NULL;
-	    line = strtok(NULL, WTPERF_CONFIG_DELIMS)) {
+	while (fgets(line, sizeof(line), fp) != NULL) {
 		linenum++;
-		/* trim the line */
+
+		/* Skip leading space. */
 		for (ltrim = line; *ltrim && isspace(*ltrim); ltrim++)
 			;
-		rtrim = &ltrim[strlen(ltrim)];
-		if (rtrim > ltrim && rtrim[-1] == '\n')
+
+		/*
+		 * Find the end of the line; if there's no trailing newline, the
+		 * the line is too long for the buffer or the file was corrupted
+		 * (there's no terminating newline in the file).
+		 */
+		for (rtrim = line; *rtrim && *rtrim != '\n'; rtrim++)
+			;
+		if (*rtrim != '\n') {
+			fprintf(stderr,
+			    "wtperf: %s: %d: configuration line too long\n",
+			    filename, linenum);
+			ret = EINVAL;
+			break;
+		}
+
+		/* Skip trailing space. */
+		while (rtrim > ltrim && isspace(rtrim[-1]))
 			rtrim--;
 
-		contline = (rtrim > ltrim && rtrim[-1] == '\\');
+		/*
+		 * If the last non-space character in the line is an escape, the
+		 * line will be continued. Checked early because the line might
+		 * otherwise be empty.
+		 */
+		contline = rtrim > ltrim && rtrim[-1] == '\\';
 		if (contline)
 			rtrim--;
 
-		comment = strchr(ltrim, '#');
-		if (comment != NULL && comment < rtrim)
+		/*
+		 * Discard anything after the first hash character. Check after
+		 * the escape character, the escape can appear after a comment.
+		 */
+		if ((comment = strchr(ltrim, '#')) != NULL)
 			rtrim = comment;
+
+		/* Skip trailing space again. */
 		while (rtrim > ltrim && isspace(rtrim[-1]))
 			rtrim--;
 
-		linelen = (size_t)(rtrim - ltrim);
-		if (linelen == 0)
-			continue;
+		/*
+		 * Check for empty lines: note that the right-hand boundary can
+		 * cross over the left-hand boundary, less-than or equal to is
+		 * the correct test.
+		 */
+		if (rtrim <= ltrim) {
+			/*
+			 * If we're continuing from this line, or we haven't
+			 * started building an option, ignore this line.
+			 */
+			if (contline || optionpos == 0)
+				continue;
+
+			/*
+			 * An empty line terminating an option we're buliding;
+			 * clean things up so we can proceed.
+			 */
+			linelen = 0;
+		} else
+			linelen = (size_t)(rtrim - ltrim);
+		ltrim[linelen] = '\0';
 
 		if (linelen + optionpos + 1 > sizeof(option)) {
-			fprintf(stderr, "wtperf: %s: %d: line overflow\n",
+			fprintf(stderr,
+			    "wtperf: %s: %d: option value overflow\n",
 			    filename, linenum);
 			ret = EINVAL;
 			break;
 		}
-		*rtrim = '\0';
-		strncpy(&option[optionpos], ltrim, linelen);
+
+		memcpy(&option[optionpos], ltrim, linelen);
 		option[optionpos + linelen] = '\0';
 		if (contline)
 			optionpos += linelen;
@@ -628,16 +645,19 @@ config_opt_file(CONFIG *cfg, const char *filename)
 			optionpos = 0;
 		}
 	}
-	if (ret == 0 && optionpos > 0) {
-		fprintf(stderr, "wtperf: %s: %d: last line continues\n",
-		    filename, linenum);
-		ret = EINVAL;
-		goto err;
+	if (ret == 0) {
+		if (ferror(fp)) {
+			fprintf(stderr, "wtperf: %s: read error\n", filename);
+			ret = errno;
+		}
+		if (optionpos > 0) {
+			fprintf(stderr, "wtperf: %s: %d: last line continues\n",
+			    filename, linenum);
+			ret = EINVAL;
+		}
 	}
 
-err:	if (fd != -1)
-		(void)close(fd);
-	free(file_buf);
+	(void)fclose(fp);
 	return (ret);
 }
 
@@ -805,7 +825,7 @@ config_consolidate(CONFIG *cfg)
 			 * as being the same key.
 			 */
 			if (strncmp(conf_line->string, test_line->string,
-			    (size_t)(string_key - conf_line->string + 1))
+			    (size_t)((string_key - conf_line->string) + 1))
 			    == 0) {
 				TAILQ_REMOVE(&cfg->config_head, conf_line, c);
 				free(conf_line->string);
diff --git a/bench/wtperf/wtperf.h b/bench/wtperf/wtperf.h
index a2b497b3142..83fab4d6028 100644
--- a/bench/wtperf/wtperf.h
+++ b/bench/wtperf/wtperf.h
@@ -30,33 +30,8 @@
 #define	HAVE_WTPERF_H
 
 #include <wt_internal.h>
-
-#ifndef _WIN32
-#include <sys/time.h>
-#endif
-#include <sys/types.h>
-#include <sys/stat.h>
-
 #include <assert.h>
-#include <ctype.h>
-#ifndef _WIN32
-#include <dirent.h>
-#endif
-#include <errno.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include <limits.h>
 #include <math.h>
-#ifndef _WIN32
-#include <pthread.h>
-#endif
-#include <stddef.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#ifndef _WIN32
-#include <unistd.h>
-#endif
 
 #ifdef _WIN32
 #include "windows_shim.h"