From 517b776042f49d2bcaa15b3bda51576cd99dbd6e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 20 Dec 2018 10:21:16 +0100 Subject: fileio: fix read_one_line() when reading bytes > 0x7F Fixes: #11218 --- src/basic/fileio.c | 12 +++++++++--- src/test/test-fileio.c | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 66e127345..e18b84299 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -755,9 +755,15 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) { (eol == EOL_NONE && previous_eol != EOL_NONE) || (eol != EOL_NONE && (previous_eol & eol) != 0)) { /* Previous char was a NUL? This is not an EOL, but the previous char was? This type of - * EOL marker has been seen right before? In either of these three cases we are - * done. But first, let's put this character back in the queue. */ - assert_se(ungetc(c, f) != EOF); + * EOL marker has been seen right before? In either of these three cases we are + * done. But first, let's put this character back in the queue. (Note that we have to + * cast this to (unsigned char) here as ungetc() expects a positive 'int', and if we + * are on an architecture where 'char' equals 'signed char' we need to ensure we don't + * pass a negative value here. That said, to complicate things further ungetc() is + * actually happy with most negative characters and implicitly casts them back to + * positive ones as needed, except for \xff (aka -1, aka EOF), which it refuses. What a + * godawful API!) */ + assert_se(ungetc((unsigned char) c, f) != EOF); break; } diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index f75b75757..08ed66d4b 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -778,7 +778,7 @@ static void test_read_line4(void) { static void test_read_nul_string(void) { static const char test[] = "string nr. 1\0" "string nr. 2\n\0" - "empty string follows\0" + "\377empty string follows\0" "\0" "final string\n is empty\0" "\0"; @@ -794,7 +794,7 @@ static void test_read_nul_string(void) { assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 14 && streq_ptr(s, "string nr. 2\n")); s = mfree(s); - assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 21 && streq_ptr(s, "empty string follows")); + assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 22 && streq_ptr(s, "\377empty string follows")); s = mfree(s); assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 1 && streq_ptr(s, "")); -- cgit v1.2.3-65-gdbad From cdce33f987c01a406e86f6aceff9d5ddea6d646a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 20 Dec 2018 11:21:36 +0100 Subject: test-fileio: add explicit check for safe_fgetc() with 0xFF --- src/test/test-fileio.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 08ed66d4b..bf918c1d1 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -611,7 +611,7 @@ static void test_tempfn(void) { } static const char chars[] = - "Aąę„”\n루"; + "Aąę„”\n루\377"; static void test_fgetc(void) { _cleanup_fclose_ FILE *f = NULL; @@ -624,11 +624,12 @@ static void test_fgetc(void) { assert_se(safe_fgetc(f, &c) == 1); assert_se(c == chars[i]); - assert_se(ungetc(c, f) != EOF); - assert_se(safe_fgetc(f, &c) == 1); + /* EOF is -1, and hence we can't push value 255 in this way */ + assert_se(ungetc(c, f) != EOF || c == EOF); + assert_se(c == EOF || safe_fgetc(f, &c) == 1); assert_se(c == chars[i]); - /* Check that ungetc doesn't care about unsigned char vs signed char */ + /* But it works when we push it properly cast */ assert_se(ungetc((unsigned char) c, f) != EOF); assert_se(safe_fgetc(f, &c) == 1); assert_se(c == chars[i]); -- cgit v1.2.3-65-gdbad