From 352f7f3a1812faba9c15bfbefcd02cb773488615 Mon Sep 17 00:00:00 2001 From: Otavio Pontes Date: Thu, 30 Aug 2018 21:57:40 +0000 Subject: [PATCH] memory: Check for allocation errors consistently Use a macro to abort on memory allocation errors in all calls to calloc, realloc or malloc --- Makefile.am | 1 + src/clean.c | 4 +--- src/download.c | 9 +++------ src/extra_files.c | 7 ++----- src/globals.c | 26 ++++++-------------------- src/hash.c | 4 +--- src/hashdump.c | 4 +--- src/hashmap.c | 7 +++---- src/helpers.c | 4 +--- src/list.c | 26 ++++---------------------- src/macros.h | 9 +++++++++ src/manifest.c | 12 +++--------- src/packs.c | 4 +--- src/search.c | 3 +++ src/subscriptions.c | 4 +--- src/swupd.h | 1 + src/verify.c | 6 ++---- src/xattrs.c | 16 ++++------------ 18 files changed, 47 insertions(+), 100 deletions(-) create mode 100644 src/macros.h diff --git a/Makefile.am b/Makefile.am index 254d9765..bb1751c1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,6 +38,7 @@ swupd_SOURCES = \ src/list.c \ src/list.h \ src/lock.c \ + src/macros.h \ src/main.c \ src/manifest.c \ src/mirror.c \ diff --git a/src/clean.c b/src/clean.c index 39165128..db462343 100644 --- a/src/clean.c +++ b/src/clean.c @@ -228,9 +228,7 @@ static char *read_mom_contents(int version) } contents = malloc(stat.st_size + 1); - if (!contents) { - goto end; - } + ON_NULL_ABORT(contents); ret = fread(contents, stat.st_size, 1, f); if (ret != 1) { diff --git a/src/download.c b/src/download.c index 1b381c2e..3a8c7a5e 100644 --- a/src/download.c +++ b/src/download.c @@ -153,9 +153,7 @@ static void free_curl_file(struct swupd_curl_parallel_handle *h, struct multi_cu void *swupd_curl_parallel_download_start(size_t max_xfer, size_t max_xfer_bottom) { struct swupd_curl_parallel_handle *h = calloc(1, sizeof(struct swupd_curl_parallel_handle)); - if (!h) { - abort(); - } + ON_NULL_ABORT(h); h->mcurl = curl_multi_init(); if (h->mcurl == NULL) { @@ -479,9 +477,8 @@ int swupd_curl_parallel_download_enqueue(void *handle, const char *url, const ch h = handle; file = calloc(1, sizeof(struct multi_curl_file)); - if (!file) { - abort(); - } + ON_NULL_ABORT(file); + file->file.path = strdup_or_die(filename); file->url = strdup_or_die(url); file->data = data; diff --git a/src/extra_files.c b/src/extra_files.c index 7625df3f..0d5e952f 100644 --- a/src/extra_files.c +++ b/src/extra_files.c @@ -75,11 +75,8 @@ static int record_filename(const char *name, const struct stat *stat __attribute char *savedname = strdup_or_die(relname); /* Only store name relative to top of area */ F = realloc(F, (nF + 1) * sizeof(*F)); /* TODO, check realloc is smart, so don't need to double myself */ - if (!F) { - fprintf(stderr, "Out of memory allocating %d filenames\n", nF); - abort(); /* For consistency with string_or_die on out of memory*/ - return -ENOMEM; - } + ON_NULL_ABORT(F); + F[nF].filename = savedname; F[nF].dir = (type == FTW_D); F[nF].in_manifest = false; /* Because we do yet know */ diff --git a/src/globals.c b/src/globals.c index 1304d832..ed97488a 100644 --- a/src/globals.c +++ b/src/globals.c @@ -78,20 +78,11 @@ timelist init_timelist(void) return head; } -static struct time *alloc_time(timelist *head) +static struct time *alloc_time() { struct time *t = calloc(1, sizeof(struct time)); - if (t == NULL) { - fprintf(stderr, "ERROR: grab_time: Failed to to allocate memory...freeing and removing timing\n"); - while (!TAILQ_EMPTY(head)) { - struct time *iter = TAILQ_FIRST(head); - TAILQ_REMOVE(head, iter, times); - free(iter); - } - /* Malloc failed...something bad happened, stop trying and let swupd attempt to finish */ - verbose_time = false; - return NULL; - } + ON_NULL_ABORT(t); + return t; } @@ -103,10 +94,8 @@ void grabtime_start(timelist *head, const char *name) } /* Only create one element for each start/stop block */ - struct time *t = alloc_time(head); - if (t == NULL) { - return; - } + struct time *t = alloc_time(); + clock_gettime(CLOCK_MONOTONIC_RAW, &t->rawstart); clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &t->procstart); t->name = name; @@ -608,10 +597,7 @@ void save_cmd(char **argv) /* +1 for null terminator */ swupd_cmd = malloc(size + 1); - if (!swupd_cmd) { - /* failed to allocate string */ - return; - } + ON_NULL_ABORT(swupd_cmd); strcpy(swupd_cmd, ""); for (int i = 0; argv[i]; i++) { diff --git a/src/hash.c b/src/hash.c index 55d9528e..109fcd58 100644 --- a/src/hash.c +++ b/src/hash.c @@ -86,9 +86,7 @@ static void hmac_sha256_for_data(char *hash, } digest_str = calloc((digest_len * 2) + 1, sizeof(char)); - if (digest_str == NULL) { - abort(); - } + ON_NULL_ABORT(digest_str); for (i = 0; i < digest_len; i++) { sprintf(&digest_str[i * 2], "%02x", (unsigned int)digest[i]); diff --git a/src/hashdump.c b/src/hashdump.c index 2e727037..3e723f68 100644 --- a/src/hashdump.c +++ b/src/hashdump.c @@ -62,9 +62,7 @@ int hashdump_main(int argc, char **argv) int ret; file = calloc(1, sizeof(struct file)); - if (!file) { - abort(); - } + ON_NULL_ABORT(file); file->use_xattrs = true; diff --git a/src/hashmap.c b/src/hashmap.c index f581ce84..2bace9b6 100644 --- a/src/hashmap.c +++ b/src/hashmap.c @@ -28,6 +28,7 @@ #include "hashmap.h" #include "list.h" +#include "macros.h" #define HASH_MASK(bits) (HASH_SIZE(bits) - 1) @@ -72,10 +73,8 @@ struct hashmap *hashmap_new(size_t capacity, hash_equal_fn_t equal, hash_fn_t ha size_t real_capacity = HASH_SIZE(mask_bits); hashmap = calloc(1, sizeof(struct hashmap) + real_capacity * sizeof(struct list *)); - if (!hashmap) { - abort(); - return NULL; - } + ON_NULL_ABORT(hashmap); + hashmap->mask_bits = mask_bits; hashmap->hash = hash; hashmap->equal = equal; diff --git a/src/helpers.c b/src/helpers.c index 5d531989..f63f7049 100644 --- a/src/helpers.c +++ b/src/helpers.c @@ -603,9 +603,7 @@ char *strdup_or_die(const char *const str) char *result; result = strdup(str); - if (!result) { - abort(); - } + ON_NULL_ABORT(result); return result; } diff --git a/src/list.c b/src/list.c index edc6e3c6..b84f2d0c 100644 --- a/src/list.c +++ b/src/list.c @@ -28,6 +28,7 @@ #include #include "list.h" +#include "macros.h" static struct list *list_append_item(struct list *list, struct list *item) { @@ -56,9 +57,7 @@ static struct list *list_alloc_item(void *data) struct list *item; item = (struct list *)malloc(sizeof(struct list)); - if (!item) { - abort(); - } + ON_NULL_ABORT(item); item->data = data; item->next = NULL; @@ -138,26 +137,12 @@ static struct list *list_merge_sort(struct list *left, unsigned int len, compari struct list *list_append_data(struct list *list, void *data) { - struct list *item = NULL; - - item = list_alloc_item(data); - if (item) { - item = list_append_item(list, item); - } - - return item; + return list_append_item(list, list_alloc_item(data)); } struct list *list_prepend_data(struct list *list, void *data) { - struct list *item = NULL; - - item = list_alloc_item(data); - if (item) { - item = list_prepend_item(list, item); - } - - return item; + return list_prepend_item(list, list_alloc_item(data)); } struct list *list_head(struct list *item) @@ -296,9 +281,6 @@ struct list *list_deep_clone_strs(struct list *list) item = list_tail(list); while (item) { clone = list_prepend_data(clone, strdup(item->data)); - if (!clone->data) { - abort(); - } item = item->prev; } diff --git a/src/macros.h b/src/macros.h new file mode 100644 index 00000000..5dfab186 --- /dev/null +++ b/src/macros.h @@ -0,0 +1,9 @@ +#ifndef __INCLUDE_GUARD_MACROS_H +#define __INCLUDE_GUARD_MACROS_H + +#define ON_NULL_ABORT(var) \ + if (!var) { \ + abort(); \ + } + +#endif diff --git a/src/manifest.c b/src/manifest.c index f0f3879d..18d7a475 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -82,9 +82,7 @@ static struct manifest *alloc_manifest(int version, char *component) struct manifest *manifest; manifest = calloc(1, sizeof(struct manifest)); - if (manifest == NULL) { - abort(); - } + ON_NULL_ABORT(manifest); manifest->version = version; manifest->component = strdup_or_die(component); @@ -235,9 +233,7 @@ static struct manifest *manifest_from_file(int version, char *component, bool he } file = calloc(1, sizeof(struct file)); - if (file == NULL) { - abort(); - } + ON_NULL_ABORT(file); c = line; c2 = strchr(c, '\t'); @@ -1310,9 +1306,7 @@ struct file **manifest_files_to_array(struct manifest *manifest) numfiles = manifest->filecount; array = malloc(sizeof(struct file *) * numfiles); - if (!array) { - abort(); - } + ON_NULL_ABORT(array); iter = list_head(manifest->files); while (iter) { diff --git a/src/packs.c b/src/packs.c index 9d3c06b9..5767b917 100644 --- a/src/packs.c +++ b/src/packs.c @@ -141,9 +141,7 @@ static int download_pack(void *download_handle, int oldversion, int newversion, string_or_die(&url, "%s/%i/pack-%s-from-%i.tar", content_url, newversion, module, oldversion); pack_data = calloc(1, sizeof(struct pack_data)); - if (!pack_data) { - abort(); - } + ON_NULL_ABORT(pack_data); pack_data->url = url; pack_data->filename = filename; diff --git a/src/search.c b/src/search.c index 54c67d6c..e51545d1 100644 --- a/src/search.c +++ b/src/search.c @@ -84,6 +84,7 @@ static void add_bundle_file_result(char *bundlename, char *filename, double scor } if (!bundle) { bundle = calloc(sizeof(struct bundle_result), 1); + ON_NULL_ABORT(bundle); results = list_append_data(results, bundle); strncpy(bundle->bundle_name, bundlename, BUNDLE_NAME_MAXLEN - 1); /* record if the bundle is tracked on the system */ @@ -91,6 +92,7 @@ static void add_bundle_file_result(char *bundlename, char *filename, double scor } file = calloc(sizeof(struct file_result), 1); + ON_NULL_ABORT(file); file->filename = strdup_or_die(filename); bundle->files = list_append_data(bundle->files, file); file->score = score; @@ -650,6 +652,7 @@ static void do_search(struct manifest *MoM, char search_type, char *search_term) /* record contentsize and includes for install size calculation */ struct bundle_result *bundle = NULL; bundle = calloc(sizeof(struct bundle_result), 1); + ON_NULL_ABORT(bundle); /* copy relevant information over for future use */ strncpy(bundle->bundle_name, subman->component, BUNDLE_NAME_MAXLEN - 1); bundle->topsize = subman->contentsize; diff --git a/src/subscriptions.c b/src/subscriptions.c index 686c4a3f..caca75fd 100644 --- a/src/subscriptions.c +++ b/src/subscriptions.c @@ -168,9 +168,7 @@ void create_and_append_subscription(struct list **subs, const char *component) struct sub *sub; sub = calloc(1, sizeof(struct sub)); - if (!sub) { - abort(); - } + ON_NULL_ABORT(sub); sub->component = strdup_or_die(component); diff --git a/src/swupd.h b/src/swupd.h index 30411fb2..9c29a0ce 100644 --- a/src/swupd.h +++ b/src/swupd.h @@ -12,6 +12,7 @@ #include #include "list.h" +#include "macros.h" #include "swupd-error.h" #ifdef __cplusplus diff --git a/src/verify.c b/src/verify.c index 39f38b7e..b4a0ec93 100644 --- a/src/verify.c +++ b/src/verify.c @@ -138,10 +138,8 @@ static bool compile_whitelist() size_t len; len = regerror(errcode, picky_whitelist, NULL, 0); error_buffer = malloc(len); - if (!error_buffer) { - fprintf(stderr, "out of memory\n"); - goto done; - } + ON_NULL_ABORT(error_buffer); + regerror(errcode, picky_whitelist, error_buffer, len); fprintf(stderr, "Invalid --picky-whitelist=%s: %s\n", cmdline_option_picky_whitelist, error_buffer); goto done; diff --git a/src/xattrs.c b/src/xattrs.c index cd0a30c6..7a385f6a 100644 --- a/src/xattrs.c +++ b/src/xattrs.c @@ -80,9 +80,7 @@ static int xattr_get_value(const char *path, const char *name, char **blob, /* realloc needed len + 1 in case we need to add final zero * to ensure consistent blob */ value = realloc(*blob, *blob_len + len + (action == XATTRS_ACTION_GET_BLOB ? 1 : 0)); - if (!value) { - abort(); - } + ON_NULL_ABORT(value); *blob = value; @@ -134,9 +132,7 @@ static const char **get_sorted_xattr_name_table(const char *names, int n) int i; table = calloc(1, n * sizeof(char *)); - if (!table) { - abort(); - } + ON_NULL_ABORT(table); for (i = 0; i < n; i++) { table[i] = names; @@ -180,9 +176,7 @@ static void xattrs_do_action(xattrs_action_type_t action, } list = calloc(1, len); - if (!list) { - abort(); - } + ON_NULL_ABORT(list); len = llistxattr(src_filename, list, len); if (len <= 0) { @@ -199,9 +193,7 @@ static void xattrs_do_action(xattrs_action_type_t action, if (action == XATTRS_ACTION_GET_BLOB) { value = calloc(1, len); - if (!value) { - abort(); - } + ON_NULL_ABORT(value); value_len = len;