Merge pull request #1335 from poettering/some-fixes

A variety of mostly unrelated fixes

7 files changed, 155 insertions, 122 deletions

diff --git a/src/basic/copy.c b/src/basic/copy.c
index b8cbe644d4..b20c178727 100644
--- a/src/basic/copy.c
+++ b/src/basic/copy.c
@@ -37,10 +37,14 @@ int copy_bytes(int fdf, int fdt, uint64_t max_bytes, bool try_reflink) {
         assert(fdt >= 0);
 
         /* Try btrfs reflinks first. */
-        if (try_reflink && max_bytes == (uint64_t) -1) {
+        if (try_reflink &&
+            max_bytes == (uint64_t) -1 &&
+            lseek(fdf, 0, SEEK_CUR) == 0 &&
+            lseek(fdt, 0, SEEK_CUR) == 0) {
+
                 r = btrfs_reflink(fdf, fdt);
                 if (r >= 0)
-                        return r;
+                        return 0; /* we copied the whole thing, hence hit EOF, return 0 */
         }
 
         for (;;) {
@@ -50,7 +54,7 @@ int copy_bytes(int fdf, int fdt, uint64_t max_bytes, bool try_reflink) {
                 if (max_bytes != (uint64_t) -1) {
 
                         if (max_bytes <= 0)
-                                return -EFBIG;
+                                return 1; /* return > 0 if we hit the max_bytes limit */
 
                         if ((uint64_t) m > max_bytes)
                                 m = (size_t) max_bytes;
@@ -75,7 +79,7 @@ int copy_bytes(int fdf, int fdt, uint64_t max_bytes, bool try_reflink) {
 
                 /* The try splice, unless we already tried */
                 if (try_splice) {
-                        n  = splice(fdf, NULL, fdt, NULL, m, 0);
+                        n = splice(fdf, NULL, fdt, NULL, m, 0);
                         if (n < 0) {
                                 if (errno != EINVAL && errno != ENOSYS)
                                         return -errno;
@@ -91,7 +95,7 @@ int copy_bytes(int fdf, int fdt, uint64_t max_bytes, bool try_reflink) {
 
                 /* As a fallback just copy bits by hand */
                 {
-                        char buf[m];
+                        uint8_t buf[m];
 
                         n = read(fdf, buf, m);
                         if (n < 0)
@@ -111,7 +115,7 @@ int copy_bytes(int fdf, int fdt, uint64_t max_bytes, bool try_reflink) {
                 }
         }
 
-        return 0;
+        return 0; /* return 0 if we hit EOF earlier than the size limit */
 }
 
 static int fd_copy_symlink(int df, const char *from, const struct stat *st, int dt, const char *to) {
diff --git a/src/basic/log.c b/src/basic/log.c
index b96afc4de4..38f42b3a6e 100644
--- a/src/basic/log.c
+++ b/src/basic/log.c
@@ -340,10 +340,10 @@ static int write_to_console(
         }
 
         if (highlight)
-                IOVEC_SET_STRING(iovec[n++], ANSI_HIGHLIGHT_RED_ON);
+                IOVEC_SET_STRING(iovec[n++], ANSI_HIGHLIGHT_RED);
         IOVEC_SET_STRING(iovec[n++], buffer);
         if (highlight)
-                IOVEC_SET_STRING(iovec[n++], ANSI_HIGHLIGHT_OFF);
+                IOVEC_SET_STRING(iovec[n++], ANSI_NORMAL);
         IOVEC_SET_STRING(iovec[n++], "\n");
 
         if (writev(console_fd, iovec, n) < 0) {
diff --git a/src/basic/macro.h b/src/basic/macro.h
index 248f7a86dd..f55d65e2f1 100644
--- a/src/basic/macro.h
+++ b/src/basic/macro.h
@@ -470,18 +470,6 @@ do {                                                                    \
 #define GID_INVALID ((gid_t) -1)
 #define MODE_INVALID ((mode_t) -1)
 
-static inline bool UID_IS_INVALID(uid_t uid) {
-        /* We consider both the old 16bit -1 user and the newer 32bit
-         * -1 user invalid, since they are or used to be incompatible
-         * with syscalls such as setresuid() or chown(). */
-
-        return uid == (uid_t) ((uint32_t) -1) || uid == (uid_t) ((uint16_t) -1);
-}
-
-static inline bool GID_IS_INVALID(gid_t gid) {
-        return gid == (gid_t) ((uint32_t) -1) || gid == (gid_t) ((uint16_t) -1);
-}
-
 #define DEFINE_TRIVIAL_CLEANUP_FUNC(type, func)                 \
         static inline void func##p(type *p) {                   \
                 if (*p)                                         \
diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index c5ef5ab0d1..dd3d525854 100644
--- a/src/basic/terminal-util.c
+++ b/src/basic/terminal-util.c
@@ -139,14 +139,14 @@ int ask_char(char *ret, const char *replies, const char *text, ...) {
                 bool need_nl = true;
 
                 if (on_tty())
-                        fputs(ANSI_HIGHLIGHT_ON, stdout);
+                        fputs(ANSI_HIGHLIGHT, stdout);
 
                 va_start(ap, text);
                 vprintf(text, ap);
                 va_end(ap);
 
                 if (on_tty())
-                        fputs(ANSI_HIGHLIGHT_OFF, stdout);
+                        fputs(ANSI_NORMAL, stdout);
 
                 fflush(stdout);
 
@@ -183,14 +183,14 @@ int ask_string(char **ret, const char *text, ...) {
                 va_list ap;
 
                 if (on_tty())
-                        fputs(ANSI_HIGHLIGHT_ON, stdout);
+                        fputs(ANSI_HIGHLIGHT, stdout);
 
                 va_start(ap, text);
                 vprintf(text, ap);
                 va_end(ap);
 
                 if (on_tty())
-                        fputs(ANSI_HIGHLIGHT_OFF, stdout);
+                        fputs(ANSI_NORMAL, stdout);
 
                 fflush(stdout);
 
diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h
index b9a3809a6c..a9e325ccb3 100644
--- a/src/basic/terminal-util.h
+++ b/src/basic/terminal-util.h
@@ -26,16 +26,22 @@
 #include "macro.h"
 #include "time-util.h"
 
-#define ANSI_HIGHLIGHT_ON "\x1B[1;39m"
-#define ANSI_RED_ON "\x1B[31m"
-#define ANSI_HIGHLIGHT_RED_ON "\x1B[1;31m"
-#define ANSI_GREEN_ON "\x1B[32m"
-#define ANSI_HIGHLIGHT_GREEN_ON "\x1B[1;32m"
-#define ANSI_HIGHLIGHT_YELLOW_ON "\x1B[1;33m"
-#define ANSI_HIGHLIGHT_BLUE_ON "\x1B[1;34m"
-#define ANSI_HIGHLIGHT_OFF "\x1B[0m"
+#define ANSI_RED "\x1B[0;31m"
+#define ANSI_GREEN "\x1B[0;32m"
+#define ANSI_UNDERLINE "\x1B[0;4m"
+#define ANSI_HIGHLIGHT "\x1B[0;1;39m"
+#define ANSI_HIGHLIGHT_RED "\x1B[0;1;31m"
+#define ANSI_HIGHLIGHT_GREEN "\x1B[0;1;32m"
+#define ANSI_HIGHLIGHT_YELLOW "\x1B[0;1;33m"
+#define ANSI_HIGHLIGHT_BLUE "\x1B[0;1;34m"
+#define ANSI_HIGHLIGHT_UNDERLINE "\x1B[0;1;4m"
+#define ANSI_NORMAL "\x1B[0m"
+
 #define ANSI_ERASE_TO_END_OF_LINE "\x1B[K"
 
+/* Set cursor to top left corner and clear screen */
+#define ANSI_HOME_CLEAR "\x1B[H\x1B[2J"
+
 int reset_terminal_fd(int fd, bool switch_to_text);
 int reset_terminal(const char *name);
 
@@ -78,28 +84,36 @@ void columns_lines_cache_reset(int _unused_ signum);
 
 bool on_tty(void);
 
+static inline const char *ansi_underline(void) {
+        return on_tty() ? ANSI_UNDERLINE : "";
+}
+
 static inline const char *ansi_highlight(void) {
-        return on_tty() ? ANSI_HIGHLIGHT_ON : "";
+        return on_tty() ? ANSI_HIGHLIGHT : "";
+}
+
+static inline const char *ansi_highlight_underline(void) {
+        return on_tty() ? ANSI_HIGHLIGHT_UNDERLINE : "";
 }
 
 static inline const char *ansi_highlight_red(void) {
-        return on_tty() ? ANSI_HIGHLIGHT_RED_ON : "";
+        return on_tty() ? ANSI_HIGHLIGHT_RED : "";
 }
 
 static inline const char *ansi_highlight_green(void) {
-        return on_tty() ? ANSI_HIGHLIGHT_GREEN_ON : "";
+        return on_tty() ? ANSI_HIGHLIGHT_GREEN : "";
 }
 
 static inline const char *ansi_highlight_yellow(void) {
-        return on_tty() ? ANSI_HIGHLIGHT_YELLOW_ON : "";
+        return on_tty() ? ANSI_HIGHLIGHT_YELLOW : "";
 }
 
 static inline const char *ansi_highlight_blue(void) {
-        return on_tty() ? ANSI_HIGHLIGHT_BLUE_ON : "";
+        return on_tty() ? ANSI_HIGHLIGHT_BLUE : "";
 }
 
-static inline const char *ansi_highlight_off(void) {
-        return on_tty() ? ANSI_HIGHLIGHT_OFF : "";
+static inline const char *ansi_normal(void) {
+        return on_tty() ? ANSI_NORMAL : "";
 }
 
 int get_ctty_devnr(pid_t pid, dev_t *d);
diff --git a/src/basic/util.c b/src/basic/util.c
index 40d9e34f85..18be0bfd5a 100644
--- a/src/basic/util.c
+++ b/src/basic/util.c
@@ -19,49 +19,48 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
-#include <string.h>
-#include <unistd.h>
+#include <ctype.h>
+#include <dirent.h>
 #include <errno.h>
-#include <stdlib.h>
-#include <signal.h>
+#include <fcntl.h>
+#include <glob.h>
+#include <grp.h>
+#include <langinfo.h>
 #include <libintl.h>
-#include <stdio.h>
-#include <syslog.h>
-#include <sched.h>
-#include <sys/resource.h>
+#include <limits.h>
+#include <linux/magic.h>
 #include <linux/sched.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <dirent.h>
-#include <sys/ioctl.h>
-#include <stdarg.h>
+#include <locale.h>
+#include <netinet/ip.h>
 #include <poll.h>
-#include <ctype.h>
-#include <sys/prctl.h>
-#include <sys/utsname.h>
 #include <pwd.h>
-#include <netinet/ip.h>
-#include <sys/wait.h>
-#include <sys/time.h>
-#include <glob.h>
-#include <grp.h>
+#include <sched.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/file.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
-#include <sys/vfs.h>
 #include <sys/mount.h>
-#include <linux/magic.h>
-#include <limits.h>
-#include <langinfo.h>
-#include <locale.h>
 #include <sys/personality.h>
-#include <sys/xattr.h>
+#include <sys/prctl.h>
+#include <sys/resource.h>
+#include <sys/stat.h>
 #include <sys/statvfs.h>
-#include <sys/file.h>
-#include <linux/fs.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/utsname.h>
+#include <sys/vfs.h>
+#include <sys/wait.h>
+#include <sys/xattr.h>
+#include <syslog.h>
+#include <unistd.h>
 
 /* When we include libgen.h because we need dirname() we immediately
- * undefine basename() since libgen.h defines it as a macro to the POSIX
- * version which is really broken. We prefer GNU basename(). */
+ * undefine basename() since libgen.h defines it as a macro to the
+ * POSIX version which is really broken. We prefer GNU basename(). */
 #include <libgen.h>
 #undef basename
 
@@ -69,31 +68,34 @@
 #include <sys/auxv.h>
 #endif
 
-#include "config.h"
-#include "macro.h"
-#include "util.h"
+/* We include linux/fs.h as last of the system headers, as it
+ * otherwise conflicts with sys/mount.h. Yay, Linux is great! */
+#include <linux/fs.h>
+
+#include "def.h"
+#include "device-nodes.h"
+#include "env-util.h"
+#include "exit-status.h"
+#include "fileio.h"
+#include "formats-util.h"
+#include "gunicode.h"
+#include "hashmap.h"
+#include "hostname-util.h"
 #include "ioprio.h"
-#include "missing.h"
 #include "log.h"
-#include "strv.h"
+#include "macro.h"
+#include "missing.h"
 #include "mkdir.h"
 #include "path-util.h"
-#include "exit-status.h"
-#include "hashmap.h"
-#include "env-util.h"
-#include "fileio.h"
-#include "device-nodes.h"
-#include "utf8.h"
-#include "gunicode.h"
-#include "virt.h"
-#include "def.h"
-#include "sparse-endian.h"
-#include "formats-util.h"
 #include "process-util.h"
 #include "random-util.h"
-#include "terminal-util.h"
-#include "hostname-util.h"
 #include "signal-util.h"
+#include "sparse-endian.h"
+#include "strv.h"
+#include "terminal-util.h"
+#include "utf8.h"
+#include "util.h"
+#include "virt.h"
 
 /* Put this test here for a lack of better place */
 assert_cc(EAGAIN == EWOULDBLOCK);
@@ -354,6 +356,17 @@ FILE* safe_fclose(FILE *f) {
         return NULL;
 }
 
+DIR* safe_closedir(DIR *d) {
+
+        if (d) {
+                PROTECT_ERRNO;
+
+                assert_se(closedir(d) >= 0 || errno != EBADF);
+        }
+
+        return NULL;
+}
+
 int unlink_noerrno(const char *path) {
         PROTECT_ERRNO;
         int r;
@@ -2133,7 +2146,13 @@ ssize_t loop_read(int fd, void *buf, size_t nbytes, bool do_poll) {
         assert(fd >= 0);
         assert(buf);
 
-        while (nbytes > 0) {
+        /* If called with nbytes == 0, let's call read() at least
+         * once, to validate the operation */
+
+        if (nbytes > (size_t) SSIZE_MAX)
+                return -EINVAL;
+
+        do {
                 ssize_t k;
 
                 k = read(fd, p, nbytes);
@@ -2147,7 +2166,7 @@ ssize_t loop_read(int fd, void *buf, size_t nbytes, bool do_poll) {
                                  * and expect that any error/EOF is reported
                                  * via read() */
 
-                                fd_wait_for_event(fd, POLLIN, USEC_INFINITY);
+                                (void) fd_wait_for_event(fd, POLLIN, USEC_INFINITY);
                                 continue;
                         }
 
@@ -2157,10 +2176,12 @@ ssize_t loop_read(int fd, void *buf, size_t nbytes, bool do_poll) {
                 if (k == 0)
                         return n;
 
+                assert((size_t) k <= nbytes);
+
                 p += k;
                 nbytes -= k;
                 n += k;
-        }
+        } while (nbytes > 0);
 
         return n;
 }
@@ -2170,9 +2191,10 @@ int loop_read_exact(int fd, void *buf, size_t nbytes, bool do_poll) {
 
         n = loop_read(fd, buf, nbytes, do_poll);
         if (n < 0)
-                return n;
+                return (int) n;
         if ((size_t) n != nbytes)
                 return -EIO;
+
         return 0;
 }
 
@@ -2182,7 +2204,8 @@ int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
         assert(fd >= 0);
         assert(buf);
 
-        errno = 0;
+        if (nbytes > (size_t) SSIZE_MAX)
+                return -EINVAL;
 
         do {
                 ssize_t k;
@@ -2197,16 +2220,18 @@ int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
                                  * and expect that any error/EOF is reported
                                  * via write() */
 
-                                fd_wait_for_event(fd, POLLOUT, USEC_INFINITY);
+                                (void) fd_wait_for_event(fd, POLLOUT, USEC_INFINITY);
                                 continue;
                         }
 
                         return -errno;
                 }
 
-                if (nbytes > 0 && k == 0) /* Can't really happen */
+                if (_unlikely_(nbytes > 0 && k == 0)) /* Can't really happen */
                         return -EIO;
 
+                assert((size_t) k <= nbytes);
+
                 p += k;
                 nbytes -= k;
         } while (nbytes > 0);
@@ -6538,7 +6563,7 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k
 
         for (i = 0; i < len; ++i)
                 if (streq_ptr(table[i], key))
-                        return (ssize_t)i;
+                        return (ssize_t) i;
 
         return -1;
 }
diff --git a/src/basic/util.h b/src/basic/util.h
index 905f375263..d53e15e6e6 100644
--- a/src/basic/util.h
+++ b/src/basic/util.h
@@ -22,30 +22,30 @@
 ***/
 
 #include <alloca.h>
+#include <dirent.h>
 #include <fcntl.h>
 #include <inttypes.h>
-#include <time.h>
+#include <limits.h>
+#include <locale.h>
+#include <mntent.h>
+#include <sched.h>
 #include <stdarg.h>
 #include <stdbool.h>
-#include <stdlib.h>
+#include <stddef.h>
 #include <stdio.h>
-#include <sched.h>
-#include <limits.h>
-#include <sys/types.h>
+#include <stdlib.h>
+#include <sys/inotify.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
-#include <dirent.h>
-#include <stddef.h>
-#include <unistd.h>
-#include <locale.h>
-#include <mntent.h>
-#include <sys/inotify.h>
 #include <sys/statfs.h>
+#include <sys/types.h>
+#include <time.h>
+#include <unistd.h>
 
+#include "formats-util.h"
 #include "macro.h"
 #include "missing.h"
 #include "time-util.h"
-#include "formats-util.h"
 
 /* What is interpreted as whitespace? */
 #define WHITESPACE " \t\n\r"
@@ -151,6 +151,7 @@ void close_many(const int fds[], unsigned n_fd);
 
 int fclose_nointr(FILE *f);
 FILE* safe_fclose(FILE *f);
+DIR* safe_closedir(DIR *f);
 
 int parse_size(const char *t, uint64_t base, uint64_t *size);
 
@@ -160,7 +161,10 @@ int parse_uid(const char *s, uid_t* ret_uid);
 #define parse_gid(s, ret_gid) parse_uid(s, ret_gid)
 
 bool uid_is_valid(uid_t uid);
-#define gid_is_valid(gid) uid_is_valid(gid)
+
+static inline bool gid_is_valid(gid_t gid) {
+        return uid_is_valid((uid_t) gid);
+}
 
 int safe_atou(const char *s, unsigned *ret_u);
 int safe_atoi(const char *s, int *ret_i);
@@ -289,9 +293,9 @@ bool chars_intersect(const char *a, const char *b) _pure_;
 
 ssize_t string_table_lookup(const char * const *table, size_t len, const char *key);
 
-#define _DEFINE_STRING_TABLE_LOOKUP_FROM_STRING(name,type,scope)                                \
-        scope inline type name##_from_string(const char *s) {                                   \
-                return (type)string_table_lookup(name##_table, ELEMENTSOF(name##_table), s);    \
+#define _DEFINE_STRING_TABLE_LOOKUP_FROM_STRING(name,type,scope)        \
+        scope type name##_from_string(const char *s) {                  \
+                return (type) string_table_lookup(name##_table, ELEMENTSOF(name##_table), s); \
         }
 
 #define _DEFINE_STRING_TABLE_LOOKUP(name,type,scope)                    \
@@ -308,17 +312,15 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k
 #define DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(name,type,max)         \
         int name##_to_string_alloc(type i, char **str) {                \
                 char *s;                                                \
-                int r;                                                  \
                 if (i < 0 || i > max)                                   \
                         return -ERANGE;                                 \
                 if (i < (type) ELEMENTSOF(name##_table)) {              \
                         s = strdup(name##_table[i]);                    \
                         if (!s)                                         \
-                                return log_oom();                       \
+                                return -ENOMEM;                         \
                 } else {                                                \
-                        r = asprintf(&s, "%i", i);                      \
-                        if (r < 0)                                      \
-                                return log_oom();                       \
+                        if (asprintf(&s, "%i", i) < 0)                  \
+                                return -ENOMEM;                         \
                 }                                                       \
                 *str = s;                                               \
                 return 0;                                               \
@@ -326,10 +328,10 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k
         type name##_from_string(const char *s) {                        \
                 type i;                                                 \
                 unsigned u = 0;                                         \
-                assert(s);                                              \
-                for (i = 0; i < (type)ELEMENTSOF(name##_table); i++)    \
-                        if (name##_table[i] &&                          \
-                            streq(name##_table[i], s))                  \
+                if (!s)                                                 \
+                        return (type) -1;                               \
+                for (i = 0; i < (type) ELEMENTSOF(name##_table); i++)   \
+                        if (streq_ptr(name##_table[i], s))              \
                                 return i;                               \
                 if (safe_atou(s, &u) >= 0 && u <= max)                  \
                         return (type) u;                                \