From 34c913485320ebc63636aac5f6e7845799322acf Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 14 Dec 2018 14:43:57 +1100 Subject: [PATCH] lib/util: Count a trailing line that doesn't end in a newline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Wed Dec 19 08:08:28 CET 2018 on sn-devel-144 (cherry picked from commit 5118985841aa0363147d552f243ab5a7d90dbdaf) --- lib/util/tests/file.c | 152 ++++++++++++++++++++++++++++++++++++++++++ lib/util/util_file.c | 6 +- 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; } -- 2.19.2