From 94062cd7c9680c5e9870f4352fcd5f0db2e51dfd Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 27 Apr 2018 14:09:31 +0200
Subject: tree-wide: be more careful with the type of array sizes

Previously we were a bit sloppy with the index and size types of arrays,
we'd regularly use unsigned. While I don't think this ever resulted in
real issues I think we should be more careful there and follow a
stricter regime: unless there's a strong reason not to use size_t for
array sizes and indexes, size_t it should be. Any allocations we do
ultimately will use size_t anyway, and converting forth and back between
unsigned and size_t will always be a source of problems.

Note that on 32bit machines "unsigned" and "size_t" are equivalent, and
on 64bit machines our arrays shouldn't grow that large anyway, and if
they do we have a problem, however that kind of overly large allocation
we have protections for usually, but for overflows we do not have that
so much, hence let's add it.

So yeah, it's a story of the current code being already "good enough",
but I think some extra type hygiene is better.

This patch tries to be comprehensive, but it probably isn't and I missed
a few cases. But I guess we can cover that later as we notice it. Among
smaller fixes, this changes:

1. strv_length()' return type becomes size_t

2. the unit file changes array size becomes size_t

3. DNS answer and query array sizes become size_t

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=76745
---
 src/basic/conf-files.c               |  2 +-
 src/basic/env-util.c                 | 12 ++++++------
 src/basic/env-util.h                 |  4 ++--
 src/basic/escape.c                   |  4 ++--
 src/basic/fd-util.c                  | 10 +++++-----
 src/basic/fd-util.h                  |  4 ++--
 src/basic/io-util.h                  |  9 ++++-----
 src/basic/locale-util.c              |  2 +-
 src/basic/log.c                      |  2 +-
 src/basic/mempool.c                  |  9 ++++-----
 src/basic/process-util.c             |  6 +++---
 src/basic/process-util.h             |  2 +-
 src/basic/random-util.c              |  2 +-
 src/basic/string-util.h              |  2 +-
 src/basic/strv.c                     | 22 +++++++++++-----------
 src/basic/strv.h                     |  6 +++---
 src/basic/time-util.c                |  8 ++++----
 src/libelogind/sd-bus/bus-internal.h |  2 +-
 src/libelogind/sd-bus/bus-message.c  | 10 +++++-----
 src/libelogind/sd-bus/bus-message.h  |  4 ++--
 src/libelogind/sd-login/sd-login.c   |  4 ++--
 21 files changed, 62 insertions(+), 64 deletions(-)

(limited to 'src')

diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c
index 8e0fb06ad..b5cad5a6e 100644
--- a/src/basic/conf-files.c
+++ b/src/basic/conf-files.c
@@ -152,8 +152,8 @@ int conf_files_insert(char ***strv, const char *root, char **dirs, const char *p
          * - do nothing if our new entry matches the existing entry,
          * - replace the existing entry if our new entry has higher priority.
          */
+        size_t i;
         char *t;
-        unsigned i;
         int r;
 
         for (i = 0; i < strv_length(*strv); i++) {
diff --git a/src/basic/env-util.c b/src/basic/env-util.c
index b5de520d4..243a76534 100644
--- a/src/basic/env-util.c
+++ b/src/basic/env-util.c
@@ -197,11 +197,11 @@ static int env_append(char **r, char ***k, char **a) {
         return 0;
 }
 
-char **strv_env_merge(unsigned n_lists, ...) {
+char **strv_env_merge(size_t n_lists, ...) {
         size_t n = 0;
         char **l, **k, **r;
         va_list ap;
-        unsigned i;
+        size_t i;
 
         /* Merges an arbitrary number of environment sets */
 
@@ -276,7 +276,7 @@ static bool env_entry_has_name(const char *entry, const char *name) {
         return *t == '=';
 }
 
-char **strv_env_delete(char **x, unsigned n_lists, ...) {
+char **strv_env_delete(char **x, size_t n_lists, ...) {
         size_t n, i = 0;
         char **k, **r;
         va_list ap;
@@ -291,7 +291,7 @@ char **strv_env_delete(char **x, unsigned n_lists, ...) {
                 return NULL;
 
         STRV_FOREACH(k, x) {
-                unsigned v;
+                size_t v;
 
                 va_start(ap, n_lists);
                 for (v = 0; v < n_lists; v++) {
@@ -677,7 +677,7 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) {
 
 char **replace_env_argv(char **argv, char **env) {
         char **ret, **i;
-        unsigned k = 0, l = 0;
+        size_t k = 0, l = 0;
 
         l = strv_length(argv);
 
@@ -691,7 +691,7 @@ char **replace_env_argv(char **argv, char **env) {
                 if ((*i)[0] == '$' && !IN_SET((*i)[1], '{', '$')) {
                         char *e;
                         char **w, **m = NULL;
-                        unsigned q;
+                        size_t q;
 
                         e = strv_env_get(env, *i+1);
                         if (e) {
diff --git a/src/basic/env-util.h b/src/basic/env-util.h
index 95b59db3d..7ac73792f 100644
--- a/src/basic/env-util.h
+++ b/src/basic/env-util.h
@@ -38,8 +38,8 @@ char **strv_env_clean_with_callback(char **l, void (*invalid_callback)(const cha
 bool strv_env_name_is_valid(char **l);
 bool strv_env_name_or_assignment_is_valid(char **l);
 
-char **strv_env_merge(unsigned n_lists, ...);
-char **strv_env_delete(char **x, unsigned n_lists, ...); /* New copy */
+char **strv_env_merge(size_t n_lists, ...);
+char **strv_env_delete(char **x, size_t n_lists, ...); /* New copy */
 
 char **strv_env_set(char **x, const char *p); /* New copy ... */
 char **strv_env_unset(char **l, const char *p); /* In place ... */
diff --git a/src/basic/escape.c b/src/basic/escape.c
index 905428f6c..87d3be8ba 100644
--- a/src/basic/escape.c
+++ b/src/basic/escape.c
@@ -188,7 +188,7 @@ int cunescape_one(const char *p, size_t length, char32_t *ret, bool *eight_bit)
                 /* C++11 style 16bit unicode */
 
                 int a[4];
-                unsigned i;
+                size_t i;
                 uint32_t c;
 
                 if (length != (size_t) -1 && length < 5)
@@ -215,7 +215,7 @@ int cunescape_one(const char *p, size_t length, char32_t *ret, bool *eight_bit)
                 /* C++11 style 32bit unicode */
 
                 int a[8];
-                unsigned i;
+                size_t i;
                 char32_t c;
 
                 if (length != (size_t) -1 && length < 9)
diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c
index f4e861dd6..9bc78b585 100644
--- a/src/basic/fd-util.c
+++ b/src/basic/fd-util.c
@@ -85,8 +85,8 @@ void safe_close_pair(int p[]) {
         p[1] = safe_close(p[1]);
 }
 
-void close_many(const int fds[], unsigned n_fd) {
-        unsigned i;
+void close_many(const int fds[], size_t n_fd) {
+        size_t i;
 
         assert(fds || n_fd <= 0);
 
@@ -180,8 +180,8 @@ int fd_cloexec(int fd, bool cloexec) {
         return 0;
 }
 
-_pure_ static bool fd_in_set(int fd, const int fdset[], unsigned n_fdset) {
-        unsigned i;
+_pure_ static bool fd_in_set(int fd, const int fdset[], size_t n_fdset) {
+        size_t i;
 
         assert(n_fdset == 0 || fdset);
 
@@ -192,7 +192,7 @@ _pure_ static bool fd_in_set(int fd, const int fdset[], unsigned n_fdset) {
         return false;
 }
 
-int close_all_fds(const int except[], unsigned n_except) {
+int close_all_fds(const int except[], size_t n_except) {
         _cleanup_closedir_ DIR *d = NULL;
         struct dirent *de;
         int r = 0;
diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h
index 588eefd50..78207ee8d 100644
--- a/src/basic/fd-util.h
+++ b/src/basic/fd-util.h
@@ -29,7 +29,7 @@ static inline int safe_close_above_stdio(int fd) {
         return safe_close(fd);
 }
 
-void close_many(const int fds[], unsigned n_fd);
+void close_many(const int fds[], size_t n_fd);
 
 int fclose_nointr(FILE *f);
 FILE* safe_fclose(FILE *f);
@@ -61,7 +61,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(DIR*, closedir);
 int fd_nonblock(int fd, bool nonblock);
 int fd_cloexec(int fd, bool cloexec);
 
-int close_all_fds(const int except[], unsigned n_except);
+int close_all_fds(const int except[], size_t n_except);
 
 #if 0 /// UNNEEDED by elogind
 int same_fd(int a, int b);
diff --git a/src/basic/io-util.h b/src/basic/io-util.h
index c34d97c65..e4717b6f3 100644
--- a/src/basic/io-util.h
+++ b/src/basic/io-util.h
@@ -28,9 +28,8 @@ int fd_wait_for_event(int fd, int event, usec_t timeout);
 
 ssize_t sparse_write(int fd, const void *p, size_t sz, size_t run_length);
 
-static inline size_t IOVEC_TOTAL_SIZE(const struct iovec *i, unsigned n) {
-        unsigned j;
-        size_t r = 0;
+static inline size_t IOVEC_TOTAL_SIZE(const struct iovec *i, size_t n) {
+        size_t j, r = 0;
 
         for (j = 0; j < n; j++)
                 r += i[j].iov_len;
@@ -38,8 +37,8 @@ static inline size_t IOVEC_TOTAL_SIZE(const struct iovec *i, unsigned n) {
         return r;
 }
 
-static inline size_t IOVEC_INCREMENT(struct iovec *i, unsigned n, size_t k) {
-        unsigned j;
+static inline size_t IOVEC_INCREMENT(struct iovec *i, size_t n, size_t k) {
+        size_t j;
 
         for (j = 0; j < n; j++) {
                 size_t sub;
diff --git a/src/basic/locale-util.c b/src/basic/locale-util.c
index 9c3f757da..0a32bca8e 100644
--- a/src/basic/locale-util.c
+++ b/src/basic/locale-util.c
@@ -71,7 +71,7 @@ static int add_locales_from_archive(Set *locales) {
         _cleanup_close_ int fd = -1;
         size_t sz = 0;
         struct stat st;
-        unsigned i;
+        size_t i;
         int r;
 
         fd = open("/usr/lib/locale/locale-archive", O_RDONLY|O_NOCTTY|O_CLOEXEC);
diff --git a/src/basic/log.c b/src/basic/log.c
index ad9d49ba9..6d5616730 100644
--- a/src/basic/log.c
+++ b/src/basic/log.c
@@ -353,8 +353,8 @@ static int write_to_console(
 
         char location[256], prefix[1 + DECIMAL_STR_MAX(int) + 2];
         struct iovec iovec[6] = {};
-        unsigned n = 0;
         bool highlight;
+        size_t n = 0;
 
         if (console_fd < 0)
                 return 0;
diff --git a/src/basic/mempool.c b/src/basic/mempool.c
index de04215ee..4be4a3d38 100644
--- a/src/basic/mempool.c
+++ b/src/basic/mempool.c
@@ -15,12 +15,12 @@
 
 struct pool {
         struct pool *next;
-        unsigned n_tiles;
-        unsigned n_used;
+        size_t n_tiles;
+        size_t n_used;
 };
 
 void* mempool_alloc_tile(struct mempool *mp) {
-        unsigned i;
+        size_t i;
 
         /* When a tile is released we add it to the list and simply
          * place the next pointer at its offset 0. */
@@ -38,8 +38,7 @@ void* mempool_alloc_tile(struct mempool *mp) {
 
         if (_unlikely_(!mp->first_pool) ||
             _unlikely_(mp->first_pool->n_used >= mp->first_pool->n_tiles)) {
-                unsigned n;
-                size_t size;
+                size_t size, n;
                 struct pool *p;
 
                 n = mp->first_pool ? mp->first_pool->n_tiles : 0;
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index a811d3dc6..94d586621 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -886,7 +886,7 @@ int getenv_for_pid(pid_t pid, const char *field, char **ret) {
 
         do {
                 char line[LINE_MAX];
-                unsigned i;
+                size_t i;
 
                 for (i = 0; i < sizeof(line)-1; i++) {
                         int c;
@@ -1387,9 +1387,9 @@ int safe_fork_full(
         return 0;
 }
 
-int fork_agent(const char *name, const int except[], unsigned n_except, pid_t *ret_pid, const char *path, ...) {
+int fork_agent(const char *name, const int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) {
         bool stdout_is_tty, stderr_is_tty;
-        unsigned n, i;
+        size_t n, i;
         va_list ap;
         char **l;
         int r;
diff --git a/src/basic/process-util.h b/src/basic/process-util.h
index ee094932a..e44f136e5 100644
--- a/src/basic/process-util.h
+++ b/src/basic/process-util.h
@@ -189,7 +189,7 @@ static inline int safe_fork(const char *name, ForkFlags flags, pid_t *ret_pid) {
         return safe_fork_full(name, NULL, 0, flags, ret_pid);
 }
 
-int fork_agent(const char *name, const int except[], unsigned n_except, pid_t *pid, const char *path, ...);
+int fork_agent(const char *name, const int except[], size_t n_except, pid_t *pid, const char *path, ...);
 
 #if SIZEOF_PID_T == 4
 /* The highest possibly (theoretic) pid_t value on this architecture. */
diff --git a/src/basic/random-util.c b/src/basic/random-util.c
index fe283050c..3205bca6f 100644
--- a/src/basic/random-util.c
+++ b/src/basic/random-util.c
@@ -35,7 +35,7 @@ int acquire_random_bytes(void *p, size_t n, bool high_quality_required) {
         static int have_syscall = -1;
 
         _cleanup_close_ int fd = -1;
-        unsigned already_done = 0;
+        size_t already_done = 0;
         int r;
 
         /* Gathers some randomness from the kernel. This call will never block. If
diff --git a/src/basic/string-util.h b/src/basic/string-util.h
index ea5f453f2..4270d3091 100644
--- a/src/basic/string-util.h
+++ b/src/basic/string-util.h
@@ -113,7 +113,7 @@ char *strjoin_real(const char *x, ...) _sentinel_;
                 const char *_appendees_[] = { a, __VA_ARGS__ };         \
                 char *_d_, *_p_;                                        \
                 size_t _len_ = 0;                                          \
-                unsigned _i_;                                           \
+                size_t _i_;                                           \
                 for (_i_ = 0; _i_ < ELEMENTSOF(_appendees_) && _appendees_[_i_]; _i_++) \
                         _len_ += strlen(_appendees_[_i_]);              \
                 _p_ = _d_ = alloca(_len_ + 1);                          \
diff --git a/src/basic/strv.c b/src/basic/strv.c
index 067c7ec4f..5620558bf 100644
--- a/src/basic/strv.c
+++ b/src/basic/strv.c
@@ -107,8 +107,8 @@ char **strv_copy(char * const *l) {
         return r;
 }
 
-unsigned strv_length(char * const *l) {
-        unsigned n = 0;
+size_t strv_length(char * const *l) {
+        size_t n = 0;
 
         if (!l)
                 return 0;
@@ -122,7 +122,7 @@ unsigned strv_length(char * const *l) {
 char **strv_new_ap(const char *x, va_list ap) {
         const char *s;
         char **a;
-        unsigned n = 0, i = 0;
+        size_t n = 0, i = 0;
         va_list aq;
 
         /* As a special trick we ignore all listed strings that equal
@@ -259,7 +259,7 @@ int strv_extend_strv_concat(char ***a, char **b, const char *suffix) {
 char **strv_split(const char *s, const char *separator) {
         const char *word, *state;
         size_t l;
-        unsigned n, i;
+        size_t n, i;
         char **r;
 
         assert(s);
@@ -290,7 +290,7 @@ char **strv_split(const char *s, const char *separator) {
 #if 0 /// UNNEEDED by elogind
 char **strv_split_newlines(const char *s) {
         char **l;
-        unsigned n;
+        size_t n;
 
         assert(s);
 
@@ -386,7 +386,7 @@ char *strv_join(char **l, const char *separator) {
 #endif // 0
 int strv_push(char ***l, char *value) {
         char **c;
-        unsigned n, m;
+        size_t n, m;
 
         if (!value)
                 return 0;
@@ -411,7 +411,7 @@ int strv_push(char ***l, char *value) {
 
 int strv_push_pair(char ***l, char *a, char *b) {
         char **c;
-        unsigned n, m;
+        size_t n, m;
 
         if (!a && !b)
                 return 0;
@@ -437,9 +437,9 @@ int strv_push_pair(char ***l, char *a, char *b) {
         return 0;
 }
 
-int strv_insert(char ***l, unsigned position, char *value) {
+int strv_insert(char ***l, size_t position, char *value) {
         char **c;
-        unsigned n, m, i;
+        size_t n, m, i;
 
         if (!value)
                 return 0;
@@ -611,7 +611,7 @@ char **strv_parse_nulstr(const char *s, size_t l) {
          */
 
         const char *p;
-        unsigned c = 0, i = 0;
+        size_t c = 0, i = 0;
         char **v;
 
         assert(s || l <= 0);
@@ -778,7 +778,7 @@ int strv_extendf(char ***l, const char *format, ...) {
 }
 
 char **strv_reverse(char **l) {
-        unsigned n, i;
+        size_t n, i;
 
         n = strv_length(l);
         if (n <= 1)
diff --git a/src/basic/strv.h b/src/basic/strv.h
index c1261269f..0b4ff30e9 100644
--- a/src/basic/strv.h
+++ b/src/basic/strv.h
@@ -32,7 +32,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(char**, strv_free_erase);
 void strv_clear(char **l);
 
 char **strv_copy(char * const *l);
-unsigned strv_length(char * const *l) _pure_;
+size_t strv_length(char * const *l) _pure_;
 
 #if 0 /// UNNEEDED by elogind
 int strv_extend_strv(char ***a, char **b, bool filter_duplicates);
@@ -45,7 +45,7 @@ int strv_extendf(char ***l, const char *format, ...) _printf_(2,0);
 int strv_extend_front(char ***l, const char *value);
 int strv_push(char ***l, char *value);
 int strv_push_pair(char ***l, char *a, char *b);
-int strv_insert(char ***l, unsigned position, char *value);
+int strv_insert(char ***l, size_t position, char *value);
 
 static inline int strv_push_prepend(char ***l, char *value) {
         return strv_insert(l, 0, value);
@@ -127,7 +127,7 @@ void strv_print(char **l);
                 if (!first)                                     \
                         _l = (char**) &first;                   \
                 else {                                          \
-                        unsigned _n;                            \
+                        size_t _n;                              \
                         va_list _ap;                            \
                                                                 \
                         _n = 1;                                 \
diff --git a/src/basic/time-util.c b/src/basic/time-util.c
index 28dc073ec..842fe7972 100644
--- a/src/basic/time-util.c
+++ b/src/basic/time-util.c
@@ -444,7 +444,7 @@ char *format_timespan(char *buf, size_t l, usec_t t, usec_t accuracy) {
                 { "us",    1               },
         };
 
-        unsigned i;
+        size_t i;
         char *p = buf;
         bool something = false;
 
@@ -625,7 +625,7 @@ static int parse_timestamp_impl(const char *t, usec_t *usec, bool with_tz) {
         time_t x;
         usec_t x_usec, plus = 0, minus = 0, ret;
         int r, weekday = -1, dst = -1;
-        unsigned i;
+        size_t i;
 
         /*
          * Allowed syntaxes:
@@ -974,7 +974,7 @@ static char* extract_multiplier(char *p, usec_t *multiplier) {
                 { "us",      1ULL            },
                 { "µs",      1ULL            },
         };
-        unsigned i;
+        size_t i;
 
         for (i = 0; i < ELEMENTSOF(table); i++) {
                 char *e;
@@ -1149,8 +1149,8 @@ int parse_nsec(const char *t, nsec_t *nsec) {
 
         for (;;) {
                 long long l, z = 0;
+                size_t n = 0, i;
                 char *e;
-                unsigned i, n = 0;
 
                 p += strspn(p, WHITESPACE);
 
diff --git a/src/libelogind/sd-bus/bus-internal.h b/src/libelogind/sd-bus/bus-internal.h
index 7c05b497d..3e80c33f9 100644
--- a/src/libelogind/sd-bus/bus-internal.h
+++ b/src/libelogind/sd-bus/bus-internal.h
@@ -265,7 +265,7 @@ struct sd_bus {
         uint64_t creds_mask;
 
         int *fds;
-        unsigned n_fds;
+        size_t n_fds;
 
         char *exec_path;
         char **exec_argv;
diff --git a/src/libelogind/sd-bus/bus-message.c b/src/libelogind/sd-bus/bus-message.c
index b8c9eed5a..c7d7c34ce 100644
--- a/src/libelogind/sd-bus/bus-message.c
+++ b/src/libelogind/sd-bus/bus-message.c
@@ -399,7 +399,7 @@ int bus_message_from_header(
                 size_t footer_accessible,
                 size_t message_size,
                 int *fds,
-                unsigned n_fds,
+                size_t n_fds,
                 const char *label,
                 size_t extra,
                 sd_bus_message **ret) {
@@ -510,7 +510,7 @@ int bus_message_from_malloc(
                 void *buffer,
                 size_t length,
                 int *fds,
-                unsigned n_fds,
+                size_t n_fds,
                 const char *label,
                 sd_bus_message **ret) {
 
@@ -1651,7 +1651,7 @@ _public_ int sd_bus_message_append_string_space(
 _public_ int sd_bus_message_append_string_iovec(
                 sd_bus_message *m,
                 const struct iovec *iov,
-                unsigned n) {
+                unsigned n /* should be size_t, but is API now… 😞 */) {
 
         size_t size;
         unsigned i;
@@ -2575,7 +2575,7 @@ _public_ int sd_bus_message_append_array_iovec(
                 sd_bus_message *m,
                 char type,
                 const struct iovec *iov,
-                unsigned n) {
+                unsigned n /* should be size_t, but is API now… 😞 */) {
 
         size_t size;
         unsigned i;
@@ -5486,7 +5486,7 @@ _public_ int sd_bus_message_set_sender(sd_bus_message *m, const char *sender) {
 int bus_message_get_blob(sd_bus_message *m, void **buffer, size_t *sz) {
         size_t total;
         void *p, *e;
-        unsigned i;
+        size_t i;
         struct bus_body_part *part;
 
         assert(m);
diff --git a/src/libelogind/sd-bus/bus-message.h b/src/libelogind/sd-bus/bus-message.h
index b7c2dc57d..b46d6ca8c 100644
--- a/src/libelogind/sd-bus/bus-message.h
+++ b/src/libelogind/sd-bus/bus-message.h
@@ -184,7 +184,7 @@ int bus_message_from_header(
                 size_t footer_accessible,
                 size_t message_size,
                 int *fds,
-                unsigned n_fds,
+                size_t n_fds,
                 const char *label,
                 size_t extra,
                 sd_bus_message **ret);
@@ -194,7 +194,7 @@ int bus_message_from_malloc(
                 void *buffer,
                 size_t length,
                 int *fds,
-                unsigned n_fds,
+                size_t n_fds,
                 const char *label,
                 sd_bus_message **ret);
 
diff --git a/src/libelogind/sd-login/sd-login.c b/src/libelogind/sd-login/sd-login.c
index 34ca3ddce..9fdb3bc73 100644
--- a/src/libelogind/sd-login/sd-login.c
+++ b/src/libelogind/sd-login/sd-login.c
@@ -432,7 +432,7 @@ static int uid_get_array(uid_t uid, const char *variable, char ***array) {
                 return -ENOMEM;
 
         strv_uniq(a);
-        r = strv_length(a);
+        r = (int) strv_length(a);
 
         if (array)
                 *array = a;
@@ -756,7 +756,7 @@ _public_ int sd_seat_get_sessions(const char *seat, char ***sessions, uid_t **ui
                 }
         }
 
-        r = strv_length(a);
+        r = (int) strv_length(a);
 
         if (sessions)
                 *sessions = TAKE_PTR(a);
-- 
cgit v1.2.3