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

diff --git a/lib/util/tests/file.c b/lib/util/tests/file.c
index f349c214f084f6ce9dc18a3c3a1e89573631d615..ca0416e20e6f5e331306f96e0e7561f515702a38 100644 (file)
--- 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 bf2f3e1a27f42135f1251a28975dbdc45cba5597..926eda240f6c196d02cd6933521b8db631f2cd3f 100644 (file)
--- a/lib/util/util_file.c
+++ b/lib/util/util_file.c
@@ -220,7 +220,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;
@@ -238,11 +238,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;
        }