basic: add assert() when doing pointer deref
Lennart reminded me in [1] that we need to add assert() in functions that do pointer access. For the simple `*p` pointer dereferences we even have an automatic coccinelle script that ensures that as part of the automatic code checks. However for deref in the `p->` style this is not supported right now and adding it to coccinelle is hard because its too slow for this kind of check. So I created a (slightly messy) tree-sitter python script to see how many asserts we are currently missing. This commit is the result of running it over the `src/basic` dir and fixing the flagged issues. I plan to tidy it up and add it to the checks too but this is orthogonal to this commit. [1] https://github.com/systemd/systemd/pull/42360#discussion_r3426964562
This commit is contained in:
committed by
Lennart Poettering
parent
cd346b8ba7
commit
bc4dfc24a5
@@ -81,6 +81,7 @@ bool argv_looks_like_help(int argc, char **argv) {
|
||||
* 3. Passing --help as any argument
|
||||
* 4. Passing -h as any argument
|
||||
*/
|
||||
assert(argc == 0 || argv);
|
||||
|
||||
if (argc <= 1)
|
||||
return true;
|
||||
|
||||
@@ -426,6 +426,8 @@ int capability_quintet_enforce(const CapabilityQuintet *q) {
|
||||
bool modified = false;
|
||||
int r;
|
||||
|
||||
assert(q);
|
||||
|
||||
if (q->ambient != CAP_MASK_UNSET ||
|
||||
q->inheritable != CAP_MASK_UNSET ||
|
||||
q->permitted != CAP_MASK_UNSET ||
|
||||
|
||||
@@ -19,6 +19,11 @@
|
||||
#if defined(__x86_64__)
|
||||
|
||||
static void cpuid(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx) {
|
||||
assert(eax);
|
||||
assert(ebx);
|
||||
assert(ecx);
|
||||
assert(edx);
|
||||
|
||||
log_debug("CPUID func %" PRIx32 " %" PRIx32, *eax, *ecx);
|
||||
__cpuid_count(*eax, *ecx, *eax, *ebx, *ecx, *edx);
|
||||
log_debug("CPUID result %" PRIx32 " %" PRIx32 " %" PRIx32 " %" PRIx32, *eax, *ebx, *ecx, *edx);
|
||||
|
||||
@@ -18,6 +18,7 @@ int parse_devnum(const char *s, dev_t *ret) {
|
||||
size_t n;
|
||||
int r;
|
||||
|
||||
assert(s);
|
||||
assert(ret);
|
||||
|
||||
n = strspn(s, DIGITS);
|
||||
|
||||
@@ -25,6 +25,7 @@ char* format_bytes_full(char *buf, size_t l, uint64_t t, FormatBytesFlag flag) {
|
||||
const suffix_table *table;
|
||||
size_t n;
|
||||
|
||||
assert(buf);
|
||||
assert_cc(ELEMENTSOF(table_iec) == ELEMENTSOF(table_si));
|
||||
|
||||
if (t == UINT64_MAX)
|
||||
|
||||
@@ -300,16 +300,20 @@ _destructor_ static void cleanup_pools(void) {
|
||||
#endif
|
||||
|
||||
static unsigned n_buckets(HashmapBase *h) {
|
||||
assert(h);
|
||||
return h->has_indirect ? h->indirect.n_buckets
|
||||
: hashmap_type_info[h->type].n_direct_buckets;
|
||||
}
|
||||
|
||||
static unsigned n_entries(HashmapBase *h) {
|
||||
assert(h);
|
||||
return h->has_indirect ? h->indirect.n_entries
|
||||
: h->n_direct_entries;
|
||||
}
|
||||
|
||||
static void n_entries_inc(HashmapBase *h) {
|
||||
assert(h);
|
||||
|
||||
if (h->has_indirect)
|
||||
h->indirect.n_entries++;
|
||||
else
|
||||
@@ -317,6 +321,8 @@ static void n_entries_inc(HashmapBase *h) {
|
||||
}
|
||||
|
||||
static void n_entries_dec(HashmapBase *h) {
|
||||
assert(h);
|
||||
|
||||
if (h->has_indirect)
|
||||
h->indirect.n_entries--;
|
||||
else
|
||||
@@ -324,11 +330,13 @@ static void n_entries_dec(HashmapBase *h) {
|
||||
}
|
||||
|
||||
static void* storage_ptr(HashmapBase *h) {
|
||||
assert(h);
|
||||
return h->has_indirect ? h->indirect.storage
|
||||
: h->direct.storage;
|
||||
}
|
||||
|
||||
static uint8_t* hash_key(HashmapBase *h) {
|
||||
assert(h);
|
||||
return h->has_indirect ? h->indirect.hash_key
|
||||
: shared_hash_key;
|
||||
}
|
||||
@@ -337,6 +345,8 @@ static unsigned base_bucket_hash(HashmapBase *h, const void *p) {
|
||||
struct siphash state;
|
||||
uint64_t hash;
|
||||
|
||||
assert(h);
|
||||
|
||||
siphash24_init(&state, hash_key(h));
|
||||
|
||||
h->hash_ops->hash(p, &state);
|
||||
@@ -348,6 +358,8 @@ static unsigned base_bucket_hash(HashmapBase *h, const void *p) {
|
||||
#define bucket_hash(h, p) base_bucket_hash(HASHMAP_BASE(h), p)
|
||||
|
||||
static void base_set_dirty(HashmapBase *h) {
|
||||
assert(h);
|
||||
|
||||
h->dirty = true;
|
||||
}
|
||||
#define hashmap_set_dirty(h) base_set_dirty(HASHMAP_BASE(h))
|
||||
@@ -372,6 +384,7 @@ static void get_hash_key(uint8_t hash_key[HASH_KEY_SIZE], bool reuse_is_ok) {
|
||||
}
|
||||
|
||||
static struct hashmap_base_entry* bucket_at(HashmapBase *h, unsigned idx) {
|
||||
assert(h);
|
||||
return CAST_ALIGN_PTR(
|
||||
struct hashmap_base_entry,
|
||||
(uint8_t *) storage_ptr(h) + idx * hashmap_type_info[h->type].entry_size);
|
||||
@@ -390,6 +403,7 @@ static struct set_entry *set_bucket_at(Set *h, unsigned idx) {
|
||||
}
|
||||
|
||||
static struct ordered_hashmap_entry* bucket_at_swap(struct swap_entries *swap, unsigned idx) {
|
||||
assert(swap);
|
||||
return &swap->e[idx - _IDX_SWAP_BEGIN];
|
||||
}
|
||||
|
||||
@@ -407,6 +421,7 @@ static struct hashmap_base_entry* bucket_at_virtual(HashmapBase *h, struct swap_
|
||||
}
|
||||
|
||||
static dib_raw_t* dib_raw_ptr(HashmapBase *h) {
|
||||
assert(h);
|
||||
return (dib_raw_t*)
|
||||
((uint8_t*) storage_ptr(h) + hashmap_type_info[h->type].entry_size * n_buckets(h));
|
||||
}
|
||||
@@ -456,6 +471,8 @@ static unsigned skip_free_buckets(HashmapBase *h, unsigned idx) {
|
||||
}
|
||||
|
||||
static void bucket_mark_free(HashmapBase *h, unsigned idx) {
|
||||
assert(h);
|
||||
|
||||
memzero(bucket_at(h, idx), hashmap_type_info[h->type].entry_size);
|
||||
bucket_set_dib(h, idx, DIB_FREE);
|
||||
}
|
||||
@@ -464,6 +481,7 @@ static void bucket_move_entry(HashmapBase *h, struct swap_entries *swap,
|
||||
unsigned from, unsigned to) {
|
||||
struct hashmap_base_entry *e_from, *e_to;
|
||||
|
||||
assert(h);
|
||||
assert(from != to);
|
||||
|
||||
e_from = bucket_at_virtual(h, swap, from);
|
||||
@@ -505,6 +523,9 @@ static unsigned prev_idx(HashmapBase *h, unsigned idx) {
|
||||
}
|
||||
|
||||
static void* entry_value(HashmapBase *h, struct hashmap_base_entry *e) {
|
||||
assert(h);
|
||||
assert(e);
|
||||
|
||||
switch (h->type) {
|
||||
|
||||
case HASHMAP_TYPE_PLAIN:
|
||||
@@ -527,6 +548,7 @@ static void base_remove_entry(HashmapBase *h, unsigned idx) {
|
||||
assert(dibs[idx] != DIB_RAW_FREE);
|
||||
|
||||
#if ENABLE_DEBUG_HASHMAP
|
||||
assert(h);
|
||||
h->debug.rem_count++;
|
||||
h->debug.last_rem_idx = idx;
|
||||
#endif
|
||||
@@ -977,6 +999,7 @@ static bool hashmap_put_robin_hood(HashmapBase *h, unsigned idx,
|
||||
unsigned dib, distance;
|
||||
|
||||
#if ENABLE_DEBUG_HASHMAP
|
||||
assert(h);
|
||||
h->debug.put_count++;
|
||||
#endif
|
||||
|
||||
|
||||
@@ -163,6 +163,8 @@ int in_addr_is_localhost_one(int family, const union in_addr_union *u) {
|
||||
}
|
||||
|
||||
bool in6_addr_is_ipv4_mapped_address(const struct in6_addr *a) {
|
||||
assert(a);
|
||||
|
||||
return a->s6_addr32[0] == 0 &&
|
||||
a->s6_addr32[1] == 0 &&
|
||||
a->s6_addr32[2] == htobe32(UINT32_C(0x0000ffff));
|
||||
@@ -728,6 +730,8 @@ int in4_addr_mask(struct in_addr *addr, unsigned char prefixlen) {
|
||||
int in6_addr_mask(struct in6_addr *addr, unsigned char prefixlen) {
|
||||
unsigned i;
|
||||
|
||||
assert(addr);
|
||||
|
||||
for (i = 0; i < 16; i++) {
|
||||
uint8_t mask;
|
||||
|
||||
|
||||
@@ -258,6 +258,8 @@ int get_locales(char ***ret) {
|
||||
_cleanup_set_free_ Set *locales = NULL;
|
||||
int r;
|
||||
|
||||
assert(ret);
|
||||
|
||||
locales = set_new(&string_hash_ops_free);
|
||||
if (!locales)
|
||||
return -ENOMEM;
|
||||
|
||||
@@ -65,6 +65,8 @@ void* mempool_alloc_tile(struct mempool *mp) {
|
||||
void* mempool_alloc0_tile(struct mempool *mp) {
|
||||
void *p;
|
||||
|
||||
assert(mp);
|
||||
|
||||
p = mempool_alloc_tile(mp);
|
||||
if (p)
|
||||
memzero(p, mp->tile_size);
|
||||
|
||||
@@ -805,7 +805,7 @@ int process_is_owned_by_uid(const PidRef *pidref, uid_t uid) {
|
||||
uid_t process_uid;
|
||||
r = pidref_get_uid(pidref, &process_uid);
|
||||
if (r < 0)
|
||||
return log_debug_errno(r, "Failed to get UID of process " PID_FMT ": %m", pidref->pid);
|
||||
return log_debug_errno(r, "Failed to get UID of process " PID_FMT ": %m", pidref ? pidref->pid : 0);
|
||||
if (process_uid == uid)
|
||||
return true;
|
||||
|
||||
|
||||
@@ -427,6 +427,8 @@ int load_os_release_pairs_with_prefix(const char *root, const char *prefix, char
|
||||
_cleanup_strv_free_ char **os_release_pairs = NULL, **os_release_pairs_prefixed = NULL;
|
||||
int r;
|
||||
|
||||
assert(ret);
|
||||
|
||||
r = load_os_release_pairs(root, &os_release_pairs);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
@@ -520,6 +520,8 @@ bool pidref_is_automatic(const PidRef *pidref) {
|
||||
}
|
||||
|
||||
void pidref_hash_func(const PidRef *pidref, struct siphash *state) {
|
||||
assert(pidref);
|
||||
|
||||
siphash24_compress_typesafe(pidref->pid, state);
|
||||
}
|
||||
|
||||
|
||||
@@ -674,6 +674,7 @@ int pidref_get_ppid_as_pidref(const PidRef *pidref, PidRef *ret) {
|
||||
pid_t ppid;
|
||||
int r;
|
||||
|
||||
POINTER_MAY_BE_NULL(pidref);
|
||||
assert(ret);
|
||||
|
||||
r = pidref_get_ppid(pidref, &ppid);
|
||||
|
||||
@@ -1289,6 +1289,8 @@ size_t sockaddr_un_len(const struct sockaddr_un *sa) {
|
||||
}
|
||||
|
||||
size_t sockaddr_len(const union sockaddr_union *sa) {
|
||||
assert(sa);
|
||||
|
||||
switch (sa->sa.sa_family) {
|
||||
case AF_INET:
|
||||
return sizeof(struct sockaddr_in);
|
||||
|
||||
@@ -737,6 +737,9 @@ bool stat_inode_unmodified(const struct stat *a, const struct stat *b) {
|
||||
* about contents of the file. The purpose here is to detect file contents changes, and nothing
|
||||
* else. */
|
||||
|
||||
assert(a);
|
||||
assert(b);
|
||||
|
||||
return stat_inode_same(a, b) &&
|
||||
a->st_mtim.tv_sec == b->st_mtim.tv_sec &&
|
||||
a->st_mtim.tv_nsec == b->st_mtim.tv_nsec &&
|
||||
@@ -773,13 +776,17 @@ int statx_mount_same(const struct statx *a, const struct statx *b) {
|
||||
}
|
||||
|
||||
usec_t statx_timestamp_load(const struct statx_timestamp *ts) {
|
||||
assert(ts);
|
||||
return timespec_load(&(const struct timespec) { .tv_sec = ts->tv_sec, .tv_nsec = ts->tv_nsec });
|
||||
}
|
||||
nsec_t statx_timestamp_load_nsec(const struct statx_timestamp *ts) {
|
||||
assert(ts);
|
||||
return timespec_load_nsec(&(const struct timespec) { .tv_sec = ts->tv_sec, .tv_nsec = ts->tv_nsec });
|
||||
}
|
||||
|
||||
void inode_hash_func(const struct stat *q, struct siphash *state) {
|
||||
assert(q);
|
||||
|
||||
siphash24_compress_typesafe(q->st_dev, state);
|
||||
siphash24_compress_typesafe(q->st_ino, state);
|
||||
|
||||
@@ -791,6 +798,9 @@ void inode_hash_func(const struct stat *q, struct siphash *state) {
|
||||
int inode_compare_func(const struct stat *a, const struct stat *b) {
|
||||
int r;
|
||||
|
||||
assert(a);
|
||||
assert(b);
|
||||
|
||||
r = CMP(a->st_dev, b->st_dev);
|
||||
if (r != 0)
|
||||
return r;
|
||||
@@ -805,6 +815,8 @@ int inode_compare_func(const struct stat *a, const struct stat *b) {
|
||||
DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(inode_hash_ops, struct stat, inode_hash_func, inode_compare_func, free);
|
||||
|
||||
void inode_unmodified_hash_func(const struct stat *q, struct siphash *state) {
|
||||
assert(q);
|
||||
|
||||
inode_hash_func(q, state);
|
||||
|
||||
siphash24_compress_typesafe(q->st_mtim.tv_sec, state);
|
||||
@@ -828,6 +840,9 @@ void inode_unmodified_hash_func(const struct stat *q, struct siphash *state) {
|
||||
int inode_unmodified_compare_func(const struct stat *a, const struct stat *b) {
|
||||
int r;
|
||||
|
||||
assert(a);
|
||||
assert(b);
|
||||
|
||||
r = inode_compare_func(a, b);
|
||||
if (r != 0)
|
||||
return r;
|
||||
|
||||
+1
-1
@@ -86,7 +86,7 @@ static void bubbleinsert(struct strbuf_node *node,
|
||||
.c = c,
|
||||
.child = node_child,
|
||||
};
|
||||
int left = 0, right = node->children_count;
|
||||
int left = 0, right = ASSERT_PTR(node)->children_count;
|
||||
|
||||
while (right > left) {
|
||||
int middle = (right + left) / 2 ;
|
||||
|
||||
@@ -11,10 +11,14 @@ const char* string_table_lookup_to_string(const char * const *table, size_t len,
|
||||
if (i < 0 || i >= (ssize_t) len)
|
||||
return NULL;
|
||||
|
||||
assert(table);
|
||||
|
||||
return table[i];
|
||||
}
|
||||
|
||||
ssize_t string_table_lookup_from_string(const char * const *table, size_t len, const char *key) {
|
||||
assert_return(table, -EINVAL);
|
||||
|
||||
if (!key)
|
||||
return -EINVAL;
|
||||
|
||||
@@ -41,6 +45,7 @@ ssize_t string_table_lookup_from_string_with_boolean(const char * const *table,
|
||||
int string_table_lookup_to_string_fallback(const char * const *table, size_t len, ssize_t i, size_t max, char **ret) {
|
||||
char *s;
|
||||
|
||||
assert(table);
|
||||
assert(ret);
|
||||
|
||||
if (i < 0 || i > (ssize_t) max)
|
||||
|
||||
@@ -163,6 +163,8 @@ char* ascii_strupper(char *s) {
|
||||
}
|
||||
|
||||
char* ascii_strlower_n(char *s, size_t n) {
|
||||
assert(n <= 0 || s);
|
||||
|
||||
if (n <= 0)
|
||||
return s;
|
||||
|
||||
|
||||
@@ -383,6 +383,7 @@ int strv_split_newlines_full(char ***ret, const char *s, ExtractFlags flags) {
|
||||
size_t n;
|
||||
int r;
|
||||
|
||||
assert(ret);
|
||||
assert(s);
|
||||
|
||||
/* Special version of strv_split_full() that splits on newlines and
|
||||
|
||||
@@ -16,6 +16,8 @@
|
||||
char* sysctl_normalize(char *s) {
|
||||
char *n;
|
||||
|
||||
assert(s);
|
||||
|
||||
n = strpbrk(s, "/.");
|
||||
|
||||
/* If the first separator is a slash, the path is
|
||||
|
||||
@@ -238,6 +238,8 @@ static CompletionResult pick_completion(const char *string, char *const*completi
|
||||
_cleanup_free_ char *found = NULL;
|
||||
bool partial = false;
|
||||
|
||||
assert(ret);
|
||||
|
||||
string = strempty(string);
|
||||
|
||||
STRV_FOREACH(c, completions) {
|
||||
@@ -527,6 +529,7 @@ int show_menu(char **x,
|
||||
const char *grey_prefix,
|
||||
bool with_numbers) {
|
||||
|
||||
assert(x);
|
||||
assert(n_columns > 0);
|
||||
|
||||
if (n_columns == SIZE_MAX)
|
||||
@@ -1673,6 +1676,8 @@ int openpt_allocate_in_namespace(
|
||||
_cleanup_close_pair_ int pair[2] = EBADF_PAIR;
|
||||
int r;
|
||||
|
||||
assert(pidref);
|
||||
|
||||
r = pidref_namespace_open(pidref, &pidnsfd, &mntnsfd, /* ret_netns_fd= */ NULL, &usernsfd, &rootfd);
|
||||
if (r < 0)
|
||||
return log_debug_errno(r, "Failed to open namespaces of PID "PID_FMT": %m", pidref->pid);
|
||||
|
||||
@@ -101,6 +101,8 @@ int fmkostemp_safe(char *pattern, const char *mode, FILE **ret_f) {
|
||||
_cleanup_close_ int fd = -EBADF;
|
||||
FILE *f;
|
||||
|
||||
assert(ret_f);
|
||||
|
||||
fd = mkostemp_safe(pattern);
|
||||
if (fd < 0)
|
||||
return fd;
|
||||
|
||||
@@ -617,7 +617,6 @@ int uid_map_search_root(pid_t pid, UIDRangeUsernsMode mode, uid_t *ret) {
|
||||
}
|
||||
|
||||
uid_t uid_range_base(const UIDRange *range) {
|
||||
|
||||
/* Returns the lowest UID in the range (notw that elements are sorted, hence we just need to look at
|
||||
* the first one that is populated. */
|
||||
|
||||
|
||||
@@ -530,6 +530,9 @@ int in_group(const char *name) {
|
||||
|
||||
int merge_gid_lists(const gid_t *list1, size_t size1, const gid_t *list2, size_t size2, gid_t **ret) {
|
||||
size_t nresult = 0;
|
||||
|
||||
assert(size1 == 0 || list1);
|
||||
assert(size2 == 0 || list2);
|
||||
assert(ret);
|
||||
|
||||
if (size2 > INT_MAX - size1)
|
||||
|
||||
@@ -424,6 +424,7 @@ char* utf16_to_utf8(const char16_t *s, size_t length /* bytes! */) {
|
||||
}
|
||||
|
||||
size_t utf16_encode_unichar(char16_t *out, char32_t c) {
|
||||
assert(out);
|
||||
|
||||
/* Note that this encodes as little-endian. */
|
||||
|
||||
|
||||
@@ -909,6 +909,8 @@ static const struct cpuid_table_entry leaf87_edx[] = {
|
||||
};
|
||||
|
||||
static bool given_flag_in_set(const char *flag, const struct cpuid_table_entry *set, size_t set_size, uint32_t val) {
|
||||
assert(set);
|
||||
|
||||
for (size_t i = 0; i < set_size; i++) {
|
||||
if ((UINT32_C(1) << set[i].flag_bit) & val &&
|
||||
streq(flag, set[i].name))
|
||||
|
||||
Reference in New Issue
Block a user