lib/util: Count a trailing line that doesn't end in a newline

If the final line of a file does not contain a newline then it isn't
included in the line count.

Change i to point to the next slot in the array instead of the current
one.  This means that that the current line won't be thrown away if no
newline is seen.

Without changing i to unsigned int, the -O3 --picky -developer build
fails with:

[ 745/4136] Compiling lib/util/util_file.c

==> /builds/samba-team/devel/samba/samba-o3.stderr <==
../../lib/util/util_file.c: In function ‘file_lines_parse’:
../../lib/util/util_file.c:251:8: error: assuming signed overflow does not occur when simplifying conditional to constant [-Werror=strict-overflow]
  while (i > 0 && ret[i-1][0] == 0) {
        ^
cc1: all warnings being treated as errors

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13717

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Dec 19 08:08:28 CET 2018 on sn-devel-144

(cherry picked from commit 5118985841aa0363147d552f243ab5a7d90dbdaf)

2 files changed, 155 insertions, 3 deletions

diff --git a/lib/util/tests/file.c b/lib/util/tests/file.c
index f349c214f08..ca0416e20e6 100644
--- a/lib/util/tests/file.c
+++ b/lib/util/tests/file.c
@@ -60,6 +60,154 @@ static bool test_file_load_save(struct torture_context *tctx)
 	return true;
 }
 
+#define TEST_DATA_WITH_NEWLINE TEST_DATA "\n"
+#define TEST_DATA_NO_NEWLINE TEST_DATA
+#define TEST_DATA_EMPTY ""
+#define TEST_DATA_BLANKS_ONLY "\n\n\n\n\n"
+#define TEST_DATA_WITH_TRAILING_BLANKS TEST_DATA TEST_DATA_BLANKS_ONLY
+
+static bool test_file_lines_load(struct torture_context *tctx)
+{
+	char **lines;
+	int numlines;
+	TALLOC_CTX *mem_ctx = tctx;
+
+	/*
+	 * Last line has trailing whitespace
+	 */
+
+	torture_assert(tctx,
+		       file_save(TEST_FILENAME,
+				 TEST_DATA_WITH_NEWLINE,
+				 strlen(TEST_DATA_WITH_NEWLINE)),
+		       "saving file");
+
+	lines = file_lines_load(TEST_FILENAME, &numlines, 0, mem_ctx);
+
+	torture_assert_int_equal(tctx, numlines, 3, "Lines");
+
+	torture_assert_mem_equal(tctx,
+				 lines[0],
+				 TEST_LINE1,
+				 strlen(TEST_LINE1),
+				 "Line 1");
+
+	torture_assert_mem_equal(tctx,
+				 lines[1],
+				 TEST_LINE2,
+				 strlen(TEST_LINE2),
+				 "Line 2");
+
+	torture_assert_mem_equal(tctx,
+				 lines[2],
+				 TEST_LINE3,
+				 strlen(TEST_LINE3),
+				 "Line 3");
+
+	unlink(TEST_FILENAME);
+
+	/*
+	 * Last line has NO trailing whitespace
+	 */
+
+	torture_assert(tctx,
+		       file_save(TEST_FILENAME,
+				 TEST_DATA_NO_NEWLINE,
+				 strlen(TEST_DATA_NO_NEWLINE)),
+		       "saving file");
+
+	lines = file_lines_load(TEST_FILENAME, &numlines, 0, mem_ctx);
+
+	torture_assert_int_equal(tctx, numlines, 3, "Lines");
+
+	torture_assert_mem_equal(tctx,
+				 lines[0],
+				 TEST_LINE1,
+				 strlen(TEST_LINE1),
+				 "Line 1");
+
+	torture_assert_mem_equal(tctx,
+				 lines[1],
+				 TEST_LINE2,
+				 strlen(TEST_LINE2),
+				 "Line 2");
+
+	torture_assert_mem_equal(tctx,
+				 lines[2],
+				 TEST_LINE3,
+				 strlen(TEST_LINE3),
+				 "Line 3");
+
+	unlink(TEST_FILENAME);
+
+	/*
+	 * Empty file
+	 */
+
+	torture_assert(tctx,
+		       file_save(TEST_FILENAME,
+				 TEST_DATA_EMPTY,
+				 strlen(TEST_DATA_EMPTY)),
+		       "saving file");
+
+	lines = file_lines_load(TEST_FILENAME, &numlines, 0, mem_ctx);
+
+	torture_assert_int_equal(tctx, numlines, 0, "Lines");
+
+	unlink(TEST_FILENAME);
+
+	/*
+	 * Just blank lines
+	 */
+
+	torture_assert(tctx,
+		       file_save(TEST_FILENAME,
+				 TEST_DATA_BLANKS_ONLY,
+				 strlen(TEST_DATA_BLANKS_ONLY)),
+		       "saving file");
+
+	lines = file_lines_load(TEST_FILENAME, &numlines, 0, mem_ctx);
+
+	torture_assert_int_equal(tctx, numlines, 0, "Lines");
+
+	unlink(TEST_FILENAME);
+
+	/*
+	 * Several trailing blank lines
+	 */
+
+	torture_assert(tctx,
+		       file_save(TEST_FILENAME,
+				 TEST_DATA_WITH_TRAILING_BLANKS,
+				 strlen(TEST_DATA_WITH_TRAILING_BLANKS)),
+		       "saving file");
+
+	lines = file_lines_load(TEST_FILENAME, &numlines, 0, mem_ctx);
+
+	torture_assert_int_equal(tctx, numlines, 3, "Lines");
+
+	torture_assert_mem_equal(tctx,
+				 lines[0],
+				 TEST_LINE1,
+				 strlen(TEST_LINE1),
+				 "Line 1");
+
+	torture_assert_mem_equal(tctx,
+				 lines[1],
+				 TEST_LINE2,
+				 strlen(TEST_LINE2),
+				 "Line 2");
+
+	torture_assert_mem_equal(tctx,
+				 lines[2],
+				 TEST_LINE3,
+				 strlen(TEST_LINE3),
+				 "Line 3");
+
+	unlink(TEST_FILENAME);
+
+	return true;
+}
 
 static bool test_afdgets(struct torture_context *tctx)
 {
@@ -102,6 +250,10 @@ struct torture_suite *torture_local_util_file(TALLOC_CTX *mem_ctx)
 	torture_suite_add_simple_test(suite, "file_load_save", 
 				      test_file_load_save);
 
+	torture_suite_add_simple_test(suite,
+				      "file_lines_load",
+				      test_file_lines_load);
+
 	torture_suite_add_simple_test(suite, "afdgets", test_afdgets);
 
 	return suite;
diff --git a/lib/util/util_file.c b/lib/util/util_file.c
index ac8206008a3..b449ce07f62 100644
--- a/lib/util/util_file.c
+++ b/lib/util/util_file.c
@@ -219,7 +219,7 @@ parse a buffer into lines
 **/
 char **file_lines_parse(char *p, size_t size, int *numlines, TALLOC_CTX *mem_ctx)
 {
-	int i;
+	unsigned int i;
 	char *s, **ret;
 
 	if (!p) return NULL;
@@ -237,11 +237,11 @@ char **file_lines_parse(char *p, size_t size, int *numlines, TALLOC_CTX *mem_ctx
 	talloc_steal(ret, p);
 
 	ret[0] = p;
-	for (s = p, i=0; s < p+size; s++) {
+	for (s = p, i=1; s < p+size; s++) {
 		if (s[0] == '\n') {
 			s[0] = 0;
-			i++;
 			ret[i] = s+1;
+			i++;
 		}
 		if (s[0] == '\r') s[0] = 0;
 	}