chiark / gitweb /
tree-wide: be more careful with the type of array sizes
authorLennart Poettering <lennart@poettering.net>
Fri, 27 Apr 2018 12:09:31 +0000 (14:09 +0200)
committerSven Eden <yamakuzure@gmx.net>
Fri, 24 Aug 2018 14:47:08 +0000 (16:47 +0200)
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
21 files changed:
src/basic/conf-files.c
src/basic/env-util.c
src/basic/env-util.h
src/basic/escape.c
src/basic/fd-util.c
src/basic/fd-util.h
src/basic/io-util.h
src/basic/locale-util.c
src/basic/log.c
src/basic/mempool.c
src/basic/process-util.c
src/basic/process-util.h
src/basic/random-util.c
src/basic/string-util.h
src/basic/strv.c
src/basic/strv.h
src/basic/time-util.c
src/libelogind/sd-bus/bus-internal.h
src/libelogind/sd-bus/bus-message.c
src/libelogind/sd-bus/bus-message.h
src/libelogind/sd-login/sd-login.c

index 8e0fb06ad9096bf2a324144da1efb37c6ce97fc3..b5cad5a6e3537d23a185529840d4b168b5a11457 100644 (file)
@@ -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++) {
index b5de520d48f44f286493860c9b4ef62a052c9f63..243a76534fe2180aae349183d2d096198c8c3be7 100644 (file)
@@ -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) {
index 95b59db3ddd61a75d6ea1c5a75cf6e2b99e6b080..7ac73792f8eeede002fe95c16602c8b28b902455 100644 (file)
@@ -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 ... */
index 905428f6c551588be5291abd6e9fdbcf5eaad25c..87d3be8baa6318163870e8504e3cd57b7f04904f 100644 (file)
@@ -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)
index f4e861dd673590bc78450b23175ffe5827d4e916..9bc78b58551582a0715fc5b4f2076f3e4fd817bb 100644 (file)
@@ -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;
index 588eefd503d4a6fc949fba65a08b7349ec4a08e5..78207ee8df6756cbdc0c1acff7853f96f2b7df00 100644 (file)
@@ -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);
index c34d97c653ed0e28967087e1b4eee424b7964120..e4717b6f30d14f4f1f1a63d75a0027e0515f388c 100644 (file)
@@ -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;
index 9c3f757da702297c0e1bb80c3234fee438149f84..0a32bca8e852e47c79e99215e58fd8d17b0934a9 100644 (file)
@@ -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);
index ad9d49ba92bcf19da0f278ca9e445346dca959c9..6d5616730dd9caf4d0eda43f12c12731cd96ce85 100644 (file)
@@ -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;
index de04215ee9ce5c650835a634df726022f170a564..4be4a3d38eb4b0f38937d9275812951a3ea37e15 100644 (file)
 
 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;
index a811d3dc69f0b89e33c2867e988b6186bc81456f..94d5866219e2ba7601037bd8843851224f1a6bba 100644 (file)
@@ -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;
index ee094932a371b41b68d84859da1961bc86776e84..e44f136e5fe7d09ed378abaa5a544af599cba402 100644 (file)
@@ -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. */
index fe283050c434468b7902f5d7f629da3be9beb867..3205bca6fc1ba35d8ad4b5ed572652d1f17a06b6 100644 (file)
@@ -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
index ea5f453f2f8b076cd0a603f147dddb74ffb70e93..4270d3091745702b5162f49f6d288fbf8cfd8052 100644 (file)
@@ -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);                          \
index 067c7ec4f678a0bfe083ab407d13ee4cc2ed9ef7..5620558bf4ae7c38ca967069813b835e530f3543 100644 (file)
@@ -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)
index c1261269fe5b8c59eef708e4b006366e51a370d3..0b4ff30e940730462e7230260b7f2d7c1ed2dfd3 100644 (file)
@@ -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;                                 \
index 28dc073eccd486632d14415ee2f216de55c7ea58..842fe79725d143ef11e07db702438650bad65b71 100644 (file)
@@ -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);
 
index 7c05b497d3b23d87b3969f6aa5da3f6737bb6e1d..3e80c33f9bf0ef6e1995ab23472088380ec0e168 100644 (file)
@@ -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;
index b8c9eed5aa319c0600766eec71dc265f7e6d59a3..c7d7c34ceedc6fa56b936d0f24343962c2e788a4 100644 (file)
@@ -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);
index b7c2dc57deb72f8c72a6fb70a5a91b285ed3a790..b46d6ca8c989f4ddb066e600f7936f32b2f62bab 100644 (file)
@@ -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);
 
index 34ca3ddcea5c521981cad0376aba66b1234c799f..9fdb3bc7350315338d80f4992a0cba58beaed951 100644 (file)
@@ -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);