From 4f9a66a32dda1d9a28f9bb3fa31c2148524bc46a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Sep 2017 17:55:53 +0200 Subject: [PATCH 01/13] fileio: add new helper call read_line() as bounded getline() replacement read_line() is much like getline(), and returns a line read from a FILE*, of arbitrary sizes. In contrast to gets() it will grow the buffer dynamically, and in contrast to getline() it will place a user-specified boundary on the line. --- src/basic/fileio.c | 77 ++++++++++++++++++++++++++++++++++++++++++ src/basic/fileio.h | 2 ++ src/test/test-fileio.c | 44 ++++++++++++++++++++++++ 3 files changed, 123 insertions(+) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 6220d1c11d..fc8776dc39 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1526,3 +1526,80 @@ int mkdtemp_malloc(const char *template, char **ret) { *ret = p; return 0; } + +int read_line(FILE *f, size_t limit, char **ret) { + _cleanup_free_ char *buffer = NULL; + size_t n = 0, allocated = 0, count = 0; + int r; + + assert(f); + + /* Something like a bounded version of getline(). + * + * Considers EOF, \n and \0 end of line delimiters, and does not include these delimiters in the string + * returned. + * + * Returns the number of bytes read from the files (i.e. including delimiters — this hence usually differs from + * the number of characters in the returned string). When EOF is hit, 0 is returned. + * + * The input parameter limit is the maximum numbers of characters in the returned string, i.e. excluding + * delimiters. If the limit is hit we fail and return -ENOBUFS. + * + * If a line shall be skipped ret may be initialized as NULL. */ + + if (ret) { + if (!GREEDY_REALLOC(buffer, allocated, 1)) + return -ENOMEM; + } + + flockfile(f); + + for (;;) { + int c; + + if (n >= limit) { + funlockfile(f); + return -ENOBUFS; + } + + errno = 0; + c = fgetc_unlocked(f); + if (c == EOF) { + /* if we read an error, and have no data to return, then propagate the error */ + if (ferror_unlocked(f) && n == 0) { + r = errno > 0 ? -errno : -EIO; + funlockfile(f); + return r; + } + + break; + } + + count++; + + if (IN_SET(c, '\n', 0)) /* Reached a delimiter */ + break; + + if (ret) { + if (!GREEDY_REALLOC(buffer, allocated, n + 2)) { + funlockfile(f); + return -ENOMEM; + } + + buffer[n] = (char) c; + } + + n++; + } + + funlockfile(f); + + if (ret) { + buffer[n] = 0; + + *ret = buffer; + buffer = NULL; + } + + return (int) count; +} diff --git a/src/basic/fileio.h b/src/basic/fileio.h index b4238746d6..9422bc759a 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -101,3 +101,5 @@ int link_tmpfile(int fd, const char *path, const char *target); int read_nul_string(FILE *f, char **ret); int mkdtemp_malloc(const char *template, char **ret); + +int read_line(FILE *f, size_t limit, char **ret); diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index b1d688c89e..375b7a8910 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -663,6 +663,49 @@ static void test_tempfn(void) { free(ret); } +static void test_read_line(void) { + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *line = NULL; + + char buffer[] = + "Some test data\n" + "With newlines, and a NUL byte\0" + "\n" + "an empty line\n" + "an ignored line\n" + "and a very long line that is supposed to be truncated, because it is so long\n"; + + f = fmemopen(buffer, sizeof(buffer), "re"); + assert_se(f); + + assert_se(read_line(f, (size_t) -1, &line) == 15 && streq(line, "Some test data")); + line = mfree(line); + + assert_se(read_line(f, 1024, &line) == 30 && streq(line, "With newlines, and a NUL byte")); + line = mfree(line); + + assert_se(read_line(f, 1024, &line) == 1 && streq(line, "")); + line = mfree(line); + + assert_se(read_line(f, 1024, &line) == 14 && streq(line, "an empty line")); + line = mfree(line); + + assert_se(read_line(f, (size_t) -1, NULL) == 16); + + assert_se(read_line(f, 16, &line) == -ENOBUFS); + line = mfree(line); + + /* read_line() stopped when it hit the limit, that means when we continue reading we'll read at the first + * character after the previous limit. Let's make use of tha to continue our test. */ + assert_se(read_line(f, 1024, &line) == 61 && streq(line, "line that is supposed to be truncated, because it is so long")); + line = mfree(line); + + assert_se(read_line(f, 1024, &line) == 1 && streq(line, "")); + line = mfree(line); + + assert_se(read_line(f, 1024, &line) == 0 && streq(line, "")); +} + int main(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); log_parse_environment(); @@ -684,6 +727,7 @@ int main(int argc, char *argv[]) { test_search_and_fopen_nulstr(); test_writing_tmpfile(); test_tempfn(); + test_read_line(); return 0; } From 189912440f6545404e84b3cd1d6ca54f1057e3e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Sep 2017 20:23:58 +0200 Subject: [PATCH 02/13] def: add new constant LONG_LINE_MAX LONG_LINE_MAX is much like LINE_MAX, but longer. As it turns out LINE_MAX at 4096 is too short for many usecases. Since the general concept of having a common maximum line length limit makes sense let's add our own, and make it larger (1MB for now). --- src/basic/def.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/basic/def.h b/src/basic/def.h index 7ce052c7ef..d3f5381e1b 100644 --- a/src/basic/def.h +++ b/src/basic/def.h @@ -84,3 +84,5 @@ "/usr/local/lib/" n "\0" \ "/usr/lib/" n "\0" \ _CONF_PATHS_SPLIT_USR(n) + +#define LONG_LINE_MAX (1U*1024U*1024U) From f4b51a2d092685c9a080e84130fec2d74c834f5c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Sep 2017 18:01:32 +0200 Subject: [PATCH 03/13] fileio: rework read_one_line_file() on top of read_line() --- src/basic/fileio.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index fc8776dc39..4d0a295760 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -30,6 +30,7 @@ #include "alloc-util.h" #include "ctype.h" +#include "def.h" #include "env-util.h" #include "escape.h" #include "fd-util.h" @@ -177,7 +178,6 @@ fail: int read_one_line_file(const char *fn, char **line) { _cleanup_fclose_ FILE *f = NULL; - char t[LINE_MAX], *c; assert(fn); assert(line); @@ -186,21 +186,7 @@ int read_one_line_file(const char *fn, char **line) { if (!f) return -errno; - if (!fgets(t, sizeof(t), f)) { - - if (ferror(f)) - return errno > 0 ? -errno : -EIO; - - t[0] = 0; - } - - c = strdup(t); - if (!c) - return -ENOMEM; - truncate_nl(c); - - *line = c; - return 0; + return read_line(f, LONG_LINE_MAX, line); } int verify_file(const char *fn, const char *blob, bool accept_extra_nl) { From 2351e44d3ed57b7a48b9e544a59c3b797ac4d216 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Sep 2017 18:25:45 +0200 Subject: [PATCH 04/13] cgroup-util: replace one use of fgets() by read_line() --- src/basic/cgroup-util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 5dea078978..040684b2b5 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -2326,7 +2326,6 @@ int cg_mask_supported(CGroupMask *ret) { int cg_kernel_controllers(Set *controllers) { _cleanup_fclose_ FILE *f = NULL; - char buf[LINE_MAX]; int r; assert(controllers); @@ -2344,7 +2343,7 @@ int cg_kernel_controllers(Set *controllers) { } /* Ignore the header line */ - (void) fgets(buf, sizeof(buf), f); + (void) read_line(f, (size_t) -1, NULL); for (;;) { char *controller; From e3f46367f577f8bd4b3a62ea0149bdcb112da573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2017 14:24:01 +0200 Subject: [PATCH 05/13] test-conf-parser: add some basic tests for config_parse() This function is pretty important, but we weren't calling it directly even once in tests. v2: add a few tests for escaping and line continuations --- src/test/test-conf-parser.c | 89 +++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c index 77fcbc0dd3..81db9d1bd7 100644 --- a/src/test/test-conf-parser.c +++ b/src/test/test-conf-parser.c @@ -18,6 +18,8 @@ ***/ #include "conf-parser.h" +#include "fd-util.h" +#include "fileio.h" #include "log.h" #include "macro.h" #include "string-util.h" @@ -227,7 +229,91 @@ static void test_config_parse_iec_uint64(void) { assert_se(config_parse_iec_uint64(NULL, "/this/file", 11, "Section", 22, "Size", 0, "4.5M", &offset, NULL) == 0); } +static const char* const config_file[] = { + "[Section]\n" + "setting1=1\n", + + "[Section]\n" + "setting1=1", /* no terminating newline */ + + "\n\n\n\n[Section]\n\n\n" + "setting1=1", /* some whitespace, no terminating newline */ + + "[Section]\n" + "[Section]\n" + "setting1=1\n" + "setting1=2\n" + "setting1=1\n", /* repeated settings */ + + "[Section]\n" + "setting1=1\\\n" /* normal continuation */ + "2\\\n" + "3\n", + + "[Section]\n" + "setting1=1\\\\\\\n" /* continuation with trailing escape symbols */ + "\\\\2\n", /* note that C requires one level of escaping, so the + * parser gets "…1 BS BS BS NL BS BS 2 NL", which + * it translates into "…1 BS BS SP BS BS 2" */ +}; + +static void test_config_parse(unsigned i, const char *s) { + char name[] = "/tmp/test-conf-parser.XXXXXX"; + int fd, r; + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *setting1 = NULL; + + const ConfigTableItem items[] = { + { "Section", "setting1", config_parse_string, 0, &setting1}, + {} + }; + + log_info("== %s[%i] ==", __func__, i); + + fd = mkostemp_safe(name); + assert_se(fd >= 0); + assert_se((size_t) write(fd, s, strlen(s)) == strlen(s)); + + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(f = fdopen(fd, "r")); + + /* + int config_parse(const char *unit, + const char *filename, + FILE *f, + const char *sections, + ConfigItemLookup lookup, + const void *table, + bool relaxed, + bool allow_include, + bool warn, + void *userdata) + */ + + r = config_parse(NULL, name, f, + "Section\0", + config_item_table_lookup, items, + false, false, true, NULL); + assert_se(r == 0); + + switch (i) { + case 0 ... 3: + assert_se(streq(setting1, "1")); + break; + + case 4: + assert_se(streq(setting1, "1 2 3")); + break; + + case 5: + assert_se(streq(setting1, "1\\\\ \\\\2")); + break; + } +} + int main(int argc, char **argv) { + unsigned i; + log_parse_environment(); log_open(); @@ -244,5 +330,8 @@ int main(int argc, char **argv) { test_config_parse_nsec(); test_config_parse_iec_uint64(); + for (i = 0; i < ELEMENTSOF(config_file); i++) + test_config_parse(i, config_file[i]); + return 0; } From e6dde451a51dc5aaa7f4d98d39b8fe735f73d2af Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Sep 2017 18:26:35 +0200 Subject: [PATCH 06/13] conf-parse: remove 4K line length limit Let's use read_line() to solve our long line limitation. Fixes #3302. --- src/shared/conf-parser.c | 43 ++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index a84b61243e..c304ae3334 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -28,8 +28,10 @@ #include "alloc-util.h" #include "conf-files.h" #include "conf-parser.h" +#include "def.h" #include "extract-word.h" #include "fd-util.h" +#include "fileio.h" #include "fs-util.h" #include "log.h" #include "macro.h" @@ -316,24 +318,44 @@ int config_parse(const char *unit, fd_warn_permissions(filename, fileno(f)); for (;;) { - char buf[LINE_MAX], *l, *p, *c = NULL, *e; + _cleanup_free_ char *buf = NULL; + char *l, *p, *c = NULL, *e; bool escaped = false; - if (!fgets(buf, sizeof buf, f)) { - if (feof(f)) - break; + r = read_line(f, LONG_LINE_MAX, &buf); + if (r == 0) + break; + if (r == -ENOBUFS) { + if (warn) + log_error_errno(r, "%s:%u: Line too long", filename, line); - return log_error_errno(errno, "Failed to read configuration file '%s': %m", filename); + return r; + } + if (r < 0) { + if (warn) + log_error_errno(r, "%s:%u: Error while reading configuration file: %m", filename, line); + + return r; } l = buf; - if (allow_bom && startswith(l, UTF8_BYTE_ORDER_MARK)) - l += strlen(UTF8_BYTE_ORDER_MARK); - allow_bom = false; + if (allow_bom) { + char *q; - truncate_nl(l); + q = startswith(buf, UTF8_BYTE_ORDER_MARK); + if (q) { + l = q; + allow_bom = false; + } + } if (continuation) { + if (strlen(continuation) + strlen(l) > LONG_LINE_MAX) { + if (warn) + log_error("%s:%u: Continuation line too long", filename, line); + return -ENOBUFS; + } + c = strappend(continuation, l); if (!c) { if (warn) @@ -387,8 +409,7 @@ int config_parse(const char *unit, if (r < 0) { if (warn) - log_warning_errno(r, "Failed to parse file '%s': %m", - filename); + log_warning_errno(r, "%s:%u: Failed to parse file: %m", filename, line); return r; } } From 620218fbd966f6514d17e163ce7c7631eef55daa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Sep 2017 18:27:03 +0200 Subject: [PATCH 07/13] update TODO --- TODO | 2 ++ 1 file changed, 2 insertions(+) diff --git a/TODO b/TODO index cabba100a5..2de99823a8 100644 --- a/TODO +++ b/TODO @@ -24,6 +24,8 @@ Janitorial Clean-ups: Features: +* replace all uses of fgets() + LINE_MAX by read_line() + * dissect: when we discover squashfs, don't claim we had a "writable" partition in systemd-dissect From f858e5148e4f36335555dfaac812197ebd3ef036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 23 Sep 2017 10:48:09 +0200 Subject: [PATCH 08/13] fileio: use _cleanup_ for FILE unlocking --- src/basic/fileio.c | 57 ++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 4d0a295760..ae5536a635 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1513,10 +1513,13 @@ int mkdtemp_malloc(const char *template, char **ret) { return 0; } +static inline void funlockfilep(FILE **f) { + funlockfile(*f); +} + int read_line(FILE *f, size_t limit, char **ret) { _cleanup_free_ char *buffer = NULL; size_t n = 0, allocated = 0, count = 0; - int r; assert(f); @@ -1538,48 +1541,42 @@ int read_line(FILE *f, size_t limit, char **ret) { return -ENOMEM; } - flockfile(f); + { + _cleanup_(funlockfilep) FILE *flocked = f; + flockfile(f); - for (;;) { - int c; + for (;;) { + int c; - if (n >= limit) { - funlockfile(f); - return -ENOBUFS; - } + if (n >= limit) + return -ENOBUFS; - errno = 0; - c = fgetc_unlocked(f); - if (c == EOF) { - /* if we read an error, and have no data to return, then propagate the error */ - if (ferror_unlocked(f) && n == 0) { - r = errno > 0 ? -errno : -EIO; - funlockfile(f); - return r; + errno = 0; + c = fgetc_unlocked(f); + if (c == EOF) { + /* if we read an error, and have no data to return, then propagate the error */ + if (ferror_unlocked(f) && n == 0) + return errno > 0 ? -errno : -EIO; + + break; } - break; - } + count++; - count++; + if (IN_SET(c, '\n', 0)) /* Reached a delimiter */ + break; - if (IN_SET(c, '\n', 0)) /* Reached a delimiter */ - break; + if (ret) { + if (!GREEDY_REALLOC(buffer, allocated, n + 2)) + return -ENOMEM; - if (ret) { - if (!GREEDY_REALLOC(buffer, allocated, n + 2)) { - funlockfile(f); - return -ENOMEM; + buffer[n] = (char) c; } - buffer[n] = (char) c; + n++; } - - n++; } - funlockfile(f); - if (ret) { buffer[n] = 0; From a12807aaeaf20ff9507cf2c1e338d5c16d528a1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2017 12:02:49 +0200 Subject: [PATCH 09/13] test-conf-parser: use _cleanup_ --- src/test/test-conf-parser.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c index 81db9d1bd7..cd538f0d40 100644 --- a/src/test/test-conf-parser.c +++ b/src/test/test-conf-parser.c @@ -27,12 +27,10 @@ #include "util.h" static void test_config_parse_path_one(const char *rvalue, const char *expected) { - char *path = NULL; + _cleanup_free_ char *path = NULL; assert_se(config_parse_path("unit", "filename", 1, "section", 1, "lvalue", 0, rvalue, &path, NULL) >= 0); assert_se(streq_ptr(expected, path)); - - free(path); } static void test_config_parse_log_level_one(const char *rvalue, int expected) { @@ -78,12 +76,10 @@ static void test_config_parse_unsigned_one(const char *rvalue, unsigned expected } static void test_config_parse_strv_one(const char *rvalue, char **expected) { - char **strv = 0; + _cleanup_strv_free_ char **strv = NULL; assert_se(config_parse_strv("unit", "filename", 1, "section", 1, "lvalue", 0, rvalue, &strv, NULL) >= 0); assert_se(strv_equal(expected, strv)); - - strv_free(strv); } static void test_config_parse_mode_one(const char *rvalue, mode_t expected) { From 8f313f4febb4df13279aaae86c846bbb142a5a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 21 Sep 2017 14:36:12 +0200 Subject: [PATCH 10/13] test-conf-parser: add tests for the new long lines, including overflow handling --- src/test/test-conf-parser.c | 41 ++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/test/test-conf-parser.c b/src/test/test-conf-parser.c index cd538f0d40..7a7de98bec 100644 --- a/src/test/test-conf-parser.c +++ b/src/test/test-conf-parser.c @@ -225,6 +225,10 @@ static void test_config_parse_iec_uint64(void) { assert_se(config_parse_iec_uint64(NULL, "/this/file", 11, "Section", 22, "Size", 0, "4.5M", &offset, NULL) == 0); } +#define x10(x) x x x x x x x x x x +#define x100(x) x10(x10(x)) +#define x1000(x) x10(x100(x)) + static const char* const config_file[] = { "[Section]\n" "setting1=1\n", @@ -251,6 +255,24 @@ static const char* const config_file[] = { "\\\\2\n", /* note that C requires one level of escaping, so the * parser gets "…1 BS BS BS NL BS BS 2 NL", which * it translates into "…1 BS BS SP BS BS 2" */ + + "\n[Section]\n\n" + "setting1=" /* a line above LINE_MAX length */ + x1000("ABCD") + "\n", + + "[Section]\n" + "setting1=" /* a line above LINE_MAX length, with continuation */ + x1000("ABCD") "\\\n" + "foobar", + + "[Section]\n" + "setting1=" /* a line above the allowed limit: 9 + 1050000 + 1 */ + x1000(x1000("x") x10("abcde")) "\n", + + "[Section]\n" + "setting1=" /* many continuation lines, together above the limit */ + x1000(x1000("x") x10("abcde") "\\\n") "xxx", }; static void test_config_parse(unsigned i, const char *s) { @@ -290,20 +312,37 @@ static void test_config_parse(unsigned i, const char *s) { "Section\0", config_item_table_lookup, items, false, false, true, NULL); - assert_se(r == 0); switch (i) { case 0 ... 3: + assert_se(r == 0); assert_se(streq(setting1, "1")); break; case 4: + assert_se(r == 0); assert_se(streq(setting1, "1 2 3")); break; case 5: + assert_se(r == 0); assert_se(streq(setting1, "1\\\\ \\\\2")); break; + + case 6: + assert_se(r == 0); + assert_se(streq(setting1, x1000("ABCD"))); + break; + + case 7: + assert_se(r == 0); + assert_se(streq(setting1, x1000("ABCD") " foobar")); + break; + + case 8 ... 9: + assert_se(r == -ENOBUFS); + assert_se(setting1 == NULL); + break; } } From 9707d55213959bb171ea810d56e9226f5ffa5466 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 Sep 2017 08:59:49 +0200 Subject: [PATCH 11/13] test-fileio: close two leaked file handles --- src/test/test-fileio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 375b7a8910..eebaa2c155 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -209,7 +209,7 @@ static void test_parse_multiline_env_file(void) { static void test_merge_env_file(void) { char t[] = "/tmp/test-fileio-XXXXXX"; int fd, r; - FILE *f; + _cleanup_fclose_ FILE *f = NULL; _cleanup_strv_free_ char **a = NULL; char **i; @@ -278,7 +278,7 @@ static void test_merge_env_file(void) { static void test_merge_env_file_invalid(void) { char t[] = "/tmp/test-fileio-XXXXXX"; int fd, r; - FILE *f; + _cleanup_fclose_ FILE *f = NULL; _cleanup_strv_free_ char **a = NULL; char **i; From 2c9de13912350f5887ccccdae9e1707512208053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 Sep 2017 09:10:48 +0200 Subject: [PATCH 12/13] test-fileio: also test read_line() with actual files Just in case the real FILE and the one from fmemopen weren't exactly the same. --- src/test/test-fileio.c | 64 +++++++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index eebaa2c155..7f22973157 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -663,21 +663,17 @@ static void test_tempfn(void) { free(ret); } -static void test_read_line(void) { - _cleanup_fclose_ FILE *f = NULL; +static const char buffer[] = + "Some test data\n" + "With newlines, and a NUL byte\0" + "\n" + "an empty line\n" + "an ignored line\n" + "and a very long line that is supposed to be truncated, because it is so long\n"; + +static void test_read_line_one_file(FILE *f) { _cleanup_free_ char *line = NULL; - char buffer[] = - "Some test data\n" - "With newlines, and a NUL byte\0" - "\n" - "an empty line\n" - "an ignored line\n" - "and a very long line that is supposed to be truncated, because it is so long\n"; - - f = fmemopen(buffer, sizeof(buffer), "re"); - assert_se(f); - assert_se(read_line(f, (size_t) -1, &line) == 15 && streq(line, "Some test data")); line = mfree(line); @@ -706,6 +702,46 @@ static void test_read_line(void) { assert_se(read_line(f, 1024, &line) == 0 && streq(line, "")); } +static void test_read_line(void) { + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *line = NULL; + + f = fmemopen((void*) buffer, sizeof(buffer), "re"); + assert_se(f); + + test_read_line_one_file(f); +} + +static void test_read_line2(void) { + char name[] = "/tmp/test-fileio.XXXXXX"; + int fd; + _cleanup_fclose_ FILE *f = NULL; + + fd = mkostemp_safe(name); + assert_se(fd >= 0); + assert_se((size_t) write(fd, buffer, sizeof(buffer)) == sizeof(buffer)); + + assert_se(lseek(fd, 0, SEEK_SET) == 0); + assert_se(f = fdopen(fd, "r")); + + test_read_line_one_file(f); +} + +static void test_read_line3(void) { + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *line = NULL; + int r; + + f = fopen("/proc/cmdline", "re"); + if (!f && IN_SET(errno, ENOENT, EPERM)) + return; + assert_se(f); + + r = read_line(f, LINE_MAX, &line); + assert_se((size_t) r == strlen(line) + 1); + assert_se(read_line(f, LINE_MAX, NULL) == 0); +} + int main(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); log_parse_environment(); @@ -728,6 +764,8 @@ int main(int argc, char *argv[]) { test_writing_tmpfile(); test_tempfn(); test_read_line(); + test_read_line2(); + test_read_line3(); return 0; } From 2e33df93dee35af986683d1226f93e0f9659de5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 24 Sep 2017 14:27:21 +0200 Subject: [PATCH 13/13] fileio: return 0 from read_one_line_file on success Fixup for f4b51a2d09. Suggested by Evgeny Vereshchagin. --- src/basic/fileio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index ae5536a635..d632a6e714 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -178,6 +178,7 @@ fail: int read_one_line_file(const char *fn, char **line) { _cleanup_fclose_ FILE *f = NULL; + int r; assert(fn); assert(line); @@ -186,7 +187,8 @@ int read_one_line_file(const char *fn, char **line) { if (!f) return -errno; - return read_line(f, LONG_LINE_MAX, line); + r = read_line(f, LONG_LINE_MAX, line); + return r < 0 ? r : 0; } int verify_file(const char *fn, const char *blob, bool accept_extra_nl) {