Compare commits

..

5 Commits

Author SHA1 Message Date
Zbigniew Jędrzejewski-Szmek 3cfb7cc507
Merge pull request #15417 from poettering/fileno-can-fail
fileio: fileno() can realistically return -1
2020-04-14 12:54:27 +02:00
Lennart Poettering 648ba0ee81 hwdb: optimize isatty()-per-line away
Fixes: #15407
2020-04-13 11:27:35 +02:00
Lennart Poettering 451fcbfc58 fileio: extend comment a bit 2020-04-13 11:27:31 +02:00
Lennart Poettering 609ae0f596 fileio: optionally allow telling read_line_full() whether we are processing a tty or not 2020-04-13 11:27:07 +02:00
Lennart Poettering 14f594b995 fileio: fileno() can realistically return -1
An stdio FILE* stream usually refers to something with a file
descriptor, but that's just "usually". It doesn't have to, when taking
fmemopen() and similar into account. Most of our calls to fileno()
assumed the call couldn't fail. In most cases this was correct, but in
some cases where we didn't know whether we work on files or memory we'd
use the returned fd as if it was unconditionally valid while it wasn't,
and passed it to a multitude of kernel syscalls. Let's fix that, and do
something reasonably smart when encountering this case.

(Running test-fileio with this patch applied will remove tons of ioctl()
calls on -1).
2020-04-13 11:26:49 +02:00
5 changed files with 67 additions and 25 deletions

View File

@ -119,7 +119,7 @@ int write_string_stream_ts(
struct timespec *ts) {
bool needs_nl;
int r;
int r, fd;
assert(f);
assert(line);
@ -127,6 +127,14 @@ int write_string_stream_ts(
if (ferror(f))
return -EIO;
if (ts) {
/* If we shall set the timestamp we need the fd. But fmemopen() streams generally don't have
* an fd. Let's fail early in that case. */
fd = fileno(f);
if (fd < 0)
return -EBADF;
}
needs_nl = !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) && !endswith(line, "\n");
if (needs_nl && (flags & WRITE_STRING_FILE_DISABLE_BUFFER)) {
@ -154,7 +162,7 @@ int write_string_stream_ts(
if (ts) {
struct timespec twice[2] = {*ts, *ts};
if (futimens(fileno(f), twice) < 0)
if (futimens(fd, twice) < 0)
return -errno;
}
@ -886,7 +894,7 @@ int fflush_and_check(FILE *f) {
}
int fflush_sync_and_check(FILE *f) {
int r;
int r, fd;
assert(f);
@ -894,10 +902,16 @@ int fflush_sync_and_check(FILE *f) {
if (r < 0)
return r;
if (fsync(fileno(f)) < 0)
/* Not all file streams have an fd associated (think: fmemopen()), let's handle this gracefully and
* assume that in that case we need no explicit syncing */
fd = fileno(f);
if (fd < 0)
return 0;
if (fsync(fd) < 0)
return -errno;
r = fsync_directory_of_file(fileno(f));
r = fsync_directory_of_file(fd);
if (r < 0)
return r;
@ -995,7 +1009,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, funlockfile);
int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
size_t n = 0, allocated = 0, count = 0;
_cleanup_free_ char *buffer = NULL;
int r, tty = -1;
int r;
assert(f);
@ -1070,13 +1084,23 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
count++;
if (eol != EOL_NONE) {
/* If we are on a tty, we can't wait for more input. But we expect only
* \n as the single EOL marker, so there is no need to wait. We check
* this condition last to avoid isatty() check if not necessary. */
/* If we are on a tty, we can't shouldn't wait for more input, because that
* generally means waiting for the user, interactively. In the case of a TTY
* we expect only \n as the single EOL marker, so we are in the lucky
* position that there is no need to wait. We check this condition last, to
* avoid isatty() check if not necessary. */
if (tty < 0)
tty = isatty(fileno(f));
if (tty > 0)
if ((flags & (READ_LINE_IS_A_TTY|READ_LINE_NOT_A_TTY)) == 0) {
int fd;
fd = fileno(f);
if (fd < 0) /* Maybe an fmemopen() stream? Handle this gracefully,
* and don't call isatty() on an invalid fd */
flags |= READ_LINE_NOT_A_TTY;
else
flags |= isatty(fd) ? READ_LINE_IS_A_TTY : READ_LINE_NOT_A_TTY;
}
if (FLAGS_SET(flags, READ_LINE_IS_A_TTY))
break;
}

View File

@ -89,6 +89,8 @@ int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space)
typedef enum ReadLineFlags {
READ_LINE_ONLY_NUL = 1 << 0,
READ_LINE_IS_A_TTY = 1 << 1,
READ_LINE_NOT_A_TTY = 1 << 2,
} ReadLineFlags;
int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret);

View File

@ -81,31 +81,34 @@ int chvt(int vt) {
int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
_cleanup_free_ char *line = NULL;
struct termios old_termios;
int r;
int r, fd;
assert(f);
assert(ret);
/* If this is a terminal, then switch canonical mode off, so that we can read a single character */
if (tcgetattr(fileno(f), &old_termios) >= 0) {
/* If this is a terminal, then switch canonical mode off, so that we can read a single
* character. (Note that fmemopen() streams do not have an fd associated with them, let's handle that
* nicely.) */
fd = fileno(f);
if (fd >= 0 && tcgetattr(fd, &old_termios) >= 0) {
struct termios new_termios = old_termios;
new_termios.c_lflag &= ~ICANON;
new_termios.c_cc[VMIN] = 1;
new_termios.c_cc[VTIME] = 0;
if (tcsetattr(fileno(f), TCSADRAIN, &new_termios) >= 0) {
if (tcsetattr(fd, TCSADRAIN, &new_termios) >= 0) {
char c;
if (t != USEC_INFINITY) {
if (fd_wait_for_event(fileno(f), POLLIN, t) <= 0) {
(void) tcsetattr(fileno(f), TCSADRAIN, &old_termios);
if (fd_wait_for_event(fd, POLLIN, t) <= 0) {
(void) tcsetattr(fd, TCSADRAIN, &old_termios);
return -ETIMEDOUT;
}
}
r = safe_fgetc(f, &c);
(void) tcsetattr(fileno(f), TCSADRAIN, &old_termios);
(void) tcsetattr(fd, TCSADRAIN, &old_termios);
if (r < 0)
return r;
if (r == 0)
@ -119,8 +122,13 @@ int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
}
}
if (t != USEC_INFINITY) {
if (fd_wait_for_event(fileno(f), POLLIN, t) <= 0)
if (t != USEC_INFINITY && fd > 0) {
/* Let's wait the specified amount of time for input. When we have no fd we skip this, under
* the assumption that this is an fmemopen() stream or so where waiting doesn't make sense
* anyway, as the data is either already in the stream or cannot possible be placed there
* while we access the stream */
if (fd_wait_for_event(fd, POLLIN, t) <= 0)
return -ETIMEDOUT;
}
@ -778,6 +786,9 @@ const char *default_term_for_tty(const char *tty) {
int fd_columns(int fd) {
struct winsize ws = {};
if (fd < 0)
return -EBADF;
if (ioctl(fd, TIOCGWINSZ, &ws) < 0)
return -errno;
@ -812,6 +823,9 @@ unsigned columns(void) {
int fd_lines(int fd) {
struct winsize ws = {};
if (fd < 0)
return -EBADF;
if (ioctl(fd, TIOCGWINSZ, &ws) < 0)
return -errno;

View File

@ -488,7 +488,7 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
size_t len;
char *pos;
r = read_line(f, LONG_LINE_MAX, &line);
r = read_line_full(f, LONG_LINE_MAX, READ_LINE_NOT_A_TTY, &line);
if (r < 0)
return r;
if (r == 0)

View File

@ -294,7 +294,7 @@ int config_parse(const char *unit,
_cleanup_fclose_ FILE *ours = NULL;
unsigned line = 0, section_line = 0;
bool section_ignored = false, bom_seen = false;
int r;
int r, fd;
assert(filename);
assert(lookup);
@ -311,7 +311,9 @@ int config_parse(const char *unit,
}
}
fd_warn_permissions(filename, fileno(f));
fd = fileno(f);
if (fd >= 0) /* stream might not have an fd, let's be careful hence */
fd_warn_permissions(filename, fd);
for (;;) {
_cleanup_free_ char *buf = NULL;