Some memory allocation improvements

- All the memory-allocation macros now auto-check for failure and exit with a failure message that incudes the caller's file and lineno info. This includes strdup(). - Added the `--max-alloc=SIZE` option to be able to override the memory allocator's sanity-check limit. It defaults to 1G (as before). Fixes bugzilla bug 12769.
RsyncProject · Jun 26, 2020 · 11eb67e · 11eb67e
1 parent 39a083b
commit 11eb67e
Show file tree

Hide file tree

Showing 31 changed files with 239 additions and 318 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -25,11 +25,22 @@ Protocol: 31 (unchanged)
  - Do not allow a negotiated checksum or compression choice of "none" unless
    the user authorized it via an environment variable or command-line option.
 
- - Improved the man page a bit more.
+ - Added the `--max-alloc=SIZE` option to be able to override the memory
+   allocator's sanity-check limit.  It defaults to 1G (as before) but the error
+   message when exceeding it specifically mentions the new option so that you
+   can differentiate an out-of-memory error from a failure of this limit.  It
+   also allows you to specify the value via the RSYNC_MAX_ALLOC environment
+   variable.
+
+ - The memory allocation functions now automatically check for a failure and
+   die when out of memory.  This eliminated some caller-side check-and-die
+   code and added some missing sanity-checking of allocations.
 
  - Preparing for an upcoming xxHash release that provides new XXH3 & XXH128
    hashing routines (disabled until their code is finalized).
 
+ - Improved the man page a bit more.
+
 ------------------------------------------------------------------------------
 <a name="3.2.1"></a>
 

diff --git a/access.c b/access.c
@@ -19,6 +19,7 @@
  */
 
 #include "rsync.h"
+#include "ifuncs.h"
 
 static int allow_forward_dns;
 
@@ -52,10 +53,8 @@ static int match_hostname(const char **host_ptr, const char *addr, const char *t
 		if (strcmp(addr, inet_ntoa(*(struct in_addr*)(hp->h_addr_list[i]))) == 0) {
 			/* If reverse lookups are off, we'll use the conf-specified
 			 * hostname in preference to UNDETERMINED. */
-			if (host == undetermined_hostname) {
-				if (!(*host_ptr = strdup(tok)))
-					*host_ptr = undetermined_hostname;
-			}
+			if (host == undetermined_hostname)
+				*host_ptr = strdup(tok);
 			return 1;
 		}
 	}
@@ -241,9 +240,6 @@ static int access_match(const char *list, const char *addr, const char **host_pt
 	char *tok;
 	char *list2 = strdup(list);
 
-	if (!list2)
-		out_of_memory("access_match");
-
 	strlower(list2);
 
 	for (tok = strtok(list2, " ,\t"); tok; tok = strtok(NULL, " ,\t")) {

diff --git a/acls.c b/acls.c
@@ -168,8 +168,6 @@ static rsync_acl *create_racl(void)
 {
 	rsync_acl *racl = new(rsync_acl);
 
-	if (!racl)
-		out_of_memory("create_racl");
 	*racl = empty_rsync_acl;
 
 	return racl;
@@ -335,8 +333,7 @@ static BOOL unpack_smb_acl(SMB_ACL_T sacl, rsync_acl *racl)
 			qsort(temp_ida_list.items, temp_ida_list.count, sizeof (id_access), id_access_sorter);
 		}
 #endif
-		if (!(racl->names.idas = new_array(id_access, temp_ida_list.count)))
-			out_of_memory("unpack_smb_acl");
+		racl->names.idas = new_array(id_access, temp_ida_list.count);
 		memcpy(racl->names.idas, temp_ida_list.items, temp_ida_list.count * sizeof (id_access));
 	} else
 		racl->names.idas = NULL;
@@ -505,9 +502,7 @@ static int get_rsync_acl(const char *fname, rsync_acl *racl,
 
 		if (cnt) {
 			char *bp = buf + 4*4;
-			id_access *ida;
-			if (!(ida = racl->names.idas = new_array(id_access, cnt)))
-				out_of_memory("get_rsync_acl");
+			id_access *ida = racl->names.idas = new_array(id_access, cnt);
 			racl->names.count = cnt;
 			for ( ; cnt--; ida++, bp += 4+4) {
 				ida->id = IVAL(bp, 0);
@@ -703,12 +698,7 @@ static uchar recv_ida_entries(int f, ida_entries *ent)
 	uchar computed_mask_bits = 0;
 	int i, count = read_varint(f);
 
-	if (count) {
-		if (!(ent->idas = new_array(id_access, count)))
-			out_of_memory("recv_ida_entries");
-	} else
-		ent->idas = NULL;
-
+	ent->idas = count ? new_array(id_access, count) : NULL;
 	ent->count = count;
 
 	for (i = 0; i < count; i++) {

diff --git a/authenticate.c b/authenticate.c
@@ -20,6 +20,7 @@
 
 #include "rsync.h"
 #include "itypes.h"
+#include "ifuncs.h"
 
 extern int read_only;
 extern char *password_file;
@@ -250,8 +251,7 @@ char *auth_server(int f_in, int f_out, int module, const char *host,
 	}
 	*pass++ = '\0';
 
-	if (!(users = strdup(users)))
-		out_of_memory("auth_server");
+	users = strdup(users);
 
 	for (tok = strtok(users, " ,\t"); tok; tok = strtok(NULL, " ,\t")) {
 		char *opts;
@@ -287,8 +287,7 @@ char *auth_server(int f_in, int f_out, int module, const char *host,
 				else {
 					gid_t *gid_array = gid_list.items;
 					auth_uid_groups_cnt = gid_list.count;
-					if ((auth_uid_groups = new_array(char *, auth_uid_groups_cnt)) == NULL)
-						out_of_memory("auth_server");
+					auth_uid_groups = new_array(char *, auth_uid_groups_cnt);
 					for (j = 0; j < auth_uid_groups_cnt; j++)
 						auth_uid_groups[j] = gid_to_group(gid_array[j]);
 				}

diff --git a/checksum.c b/checksum.c
@@ -271,8 +271,6 @@ void get_checksum2(char *buf, int32 len, char *sum)
 				free(buf1);
 			buf1 = new_array(char, len+4);
 			len1 = len;
-			if (!buf1)
-				out_of_memory("get_checksum2");
 		}
 
 		memcpy(buf1, buf, len);

diff --git a/clientserver.c b/clientserver.c
@@ -235,8 +235,7 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char
 	else
 		modlen = p - *argv;
 
-	if (!(modname = new_array(char, modlen+1+1))) /* room for '/' & '\0' */
-		out_of_memory("start_inband_exchange");
+	modname = new_array(char, modlen+1+1); /* room for '/' & '\0' */
 	strlcpy(modname, *argv, modlen + 1);
 	modname[modlen] = '/';
 	modname[modlen+1] = '\0';
@@ -1233,8 +1232,7 @@ int start_daemon(int f_in, int f_out)
 			io_printf(f_out, "@ERROR: invalid early_input length\n");
 			return -1;
 		}
-		if (!(early_input = new_array(char, early_input_len)))
-			out_of_memory("exchange_protocols");
+		early_input = new_array(char, early_input_len);
 		read_buf(f_in, early_input, early_input_len);
 
 		if (!read_line_old(f_in, line, sizeof line, 0))

diff --git a/compat.c b/compat.c
@@ -243,8 +243,7 @@ static void init_nno_saw(struct name_num_obj *nno, int val)
 	}
 
 	if (!nno->saw) {
-		if (!(nno->saw = new_array0(uchar, nno->saw_len)))
-			out_of_memory("init_nno_saw");
+		nno->saw = new_array0(uchar, nno->saw_len);
 
 		/* We'll take this opportunity to make sure that the main_name values are set right. */
 		for (cnt = 1, nni = nno->list; nni->name; nni++, cnt++) {

diff --git a/exclude.c b/exclude.c
@@ -22,6 +22,7 @@
 
 #include "rsync.h"
 #include "default-cvsignore.h"
+#include "ifuncs.h"
 
 extern int am_server;
 extern int am_sender;
@@ -200,8 +201,7 @@ static void add_rule(filter_rule_list *listp, const char *pat, unsigned int pat_
 	} else
 		suf_len = 0;
 
-	if (!(rule->pattern = new_array(char, pre_len + pat_len + suf_len + 1)))
-		out_of_memory("add_rule");
+	rule->pattern = new_array(char, pre_len + pat_len + suf_len + 1);
 	if (pre_len) {
 		memcpy(rule->pattern, dirbuf + module_dirlen, pre_len);
 		for (cp = rule->pattern; cp < rule->pattern + pre_len; cp++) {
@@ -262,19 +262,14 @@ static void add_rule(filter_rule_list *listp, const char *pat, unsigned int pat_
 			}
 		}
 
-		if (!(lp = new_array0(filter_rule_list, 1)))
-			out_of_memory("add_rule");
+		lp = new_array0(filter_rule_list, 1);
 		if (asprintf(&lp->debug_type, " [per-dir %s]", cp) < 0)
 			out_of_memory("add_rule");
 		rule->u.mergelist = lp;
 
 		if (mergelist_cnt == mergelist_size) {
 			mergelist_size += 5;
-			mergelist_parents = realloc_array(mergelist_parents,
-						filter_rule *,
-						mergelist_size);
-			if (!mergelist_parents)
-				out_of_memory("add_rule");
+			mergelist_parents = realloc_array(mergelist_parents, filter_rule *, mergelist_size);
 		}
 		if (DEBUG_GTE(FILTER, 2)) {
 			rprintf(FINFO, "[%s] activating mergelist #%d%s\n",
@@ -498,8 +493,6 @@ void *push_local_filters(const char *dir, unsigned int dirlen)
 	push = (struct local_filter_state *)new_array(char,
 			  sizeof (struct local_filter_state)
 			+ (mergelist_cnt-1) * sizeof (filter_rule_list));
-	if (!push)
-		out_of_memory("push_local_filters");
 
 	push->mergelist_cnt = mergelist_cnt;
 	for (i = 0; i < mergelist_cnt; i++) {
@@ -822,8 +815,7 @@ static filter_rule *parse_rule_tok(const char **rulestr_ptr,
 	if (!*s)
 		return NULL;
 
-	if (!(rule = new0(filter_rule)))
-		out_of_memory("parse_rule_tok");
+	rule = new0(filter_rule);
 
 	/* Inherit from the template.  Don't inherit FILTRULES_SIDES; we check
 	 * that later. */
@@ -1125,8 +1117,7 @@ void parse_filter_str(filter_rule_list *listp, const char *rulestr,
 				const char *name;
 				filter_rule *excl_self;
 
-				if (!(excl_self = new0(filter_rule)))
-					out_of_memory("parse_filter_str");
+				excl_self = new0(filter_rule);
 				/* Find the beginning of the basename and add an exclude for it. */
 				for (name = pat + pat_len; name > pat && name[-1] != '/'; name--) {}
 				add_rule(listp, name, (pat + pat_len) - name, excl_self, 0);

diff --git a/fileio.c b/fileio.c
@@ -157,8 +157,6 @@ int write_file(int f, int use_seek, OFF_T offset, const char *buf, int len)
 				wf_writeBufSize = WRITE_SIZE * 8;
 				wf_writeBufCnt  = 0;
 				wf_writeBuf = new_array(char, wf_writeBufSize);
-				if (!wf_writeBuf)
-					out_of_memory("write_file");
 			}
 			r1 = (int)MIN((size_t)len, wf_writeBufSize - wf_writeBufCnt);
 			if (r1) {
@@ -217,8 +215,7 @@ struct map_struct *map_file(int fd, OFF_T len, int32 read_size, int32 blk_size)
 {
 	struct map_struct *map;
 
-	if (!(map = new0(struct map_struct)))
-		out_of_memory("map_file");
+	map = new0(struct map_struct);
 
 	if (blk_size && (read_size % blk_size))
 		read_size += blk_size - (read_size % blk_size);
@@ -261,8 +258,6 @@ char *map_ptr(struct map_struct *map, OFF_T offset, int32 len)
 	/* make sure we have allocated enough memory for the window */
 	if (window_size > map->p_size) {
 		map->p = realloc_array(map->p, char, window_size);
-		if (!map->p)
-			out_of_memory("map_ptr");
 		map->p_size = window_size;
 	}
 

diff --git a/flist.c b/flist.c
@@ -301,8 +301,7 @@ static void flist_expand(struct file_list *flist, int extra)
 	if (flist->malloced < flist->used + extra)
 		flist->malloced = flist->used + extra;
 
-	new_ptr = realloc_array(flist->files, struct file_struct *,
-				flist->malloced);
+	new_ptr = realloc_array(flist->files, struct file_struct *, flist->malloced);
 
 	if (DEBUG_GTE(FLIST, 1) && flist->malloced != FLIST_START) {
 		rprintf(FCLIENT, "[%s] expand file_list pointer array to %s bytes, did%s move\n",
@@ -1335,10 +1334,8 @@ struct file_struct *make_file(const char *fname, struct file_list *flist,
 		  + linkname_len;
 	if (pool)
 		bp = pool_alloc(pool, alloc_len, "make_file");
-	else {
-		if (!(bp = new_array(char, alloc_len)))
-			out_of_memory("make_file");
-	}
+	else
+		bp = new_array(char, alloc_len);
 
 	memset(bp, 0, extra_len + FILE_STRUCT_LEN);
 	bp += extra_len;
@@ -1661,8 +1658,7 @@ static void fsort(struct file_struct **fp, size_t num)
 	if (use_qsort)
 		qsort(fp, num, PTR_SIZE, file_compare);
 	else {
-		struct file_struct **tmp = new_array(struct file_struct *,
-						     (num+1) / 2);
+		struct file_struct **tmp = new_array(struct file_struct *, (num+1) / 2);
 		fsort_tmp(fp, num, tmp);
 		free(tmp);
 	}
@@ -1895,13 +1891,11 @@ static void send_implied_dirs(int f, struct file_list *flist, char *fname,
 	len = strlen(limit+1);
 	memcpy(&relname_list, F_DIR_RELNAMES_P(lastpath_struct), sizeof relname_list);
 	if (!relname_list) {
-		if (!(relname_list = new0(item_list)))
-			out_of_memory("send_implied_dirs");
+		relname_list = new0(item_list);
 		memcpy(F_DIR_RELNAMES_P(lastpath_struct), &relname_list, sizeof relname_list);
 	}
 	rnpp = EXPAND_ITEM_LIST(relname_list, relnamecache *, 32);
-	if (!(*rnpp = (relnamecache*)new_array(char, sizeof (relnamecache) + len)))
-		out_of_memory("send_implied_dirs");
+	*rnpp = (relnamecache*)new_array(char, sizeof (relnamecache) + len);
 	(*rnpp)->name_type = name_type;
 	strlcpy((*rnpp)->fname, limit+1, len + 1);
 
@@ -2059,8 +2053,7 @@ void send_extra_file_list(int f, int at_least)
 		}
 
 		if (need_unsorted_flist) {
-			if (!(flist->sorted = new_array(struct file_struct *, flist->used)))
-				out_of_memory("send_extra_file_list");
+			flist->sorted = new_array(struct file_struct *, flist->used);
 			memcpy(flist->sorted, flist->files,
 			       flist->used * sizeof (struct file_struct*));
 		} else
@@ -2414,8 +2407,7 @@ struct file_list *send_file_list(int f, int argc, char *argv[])
 	 * recursion mode, the sender marks duplicate dirs so that it can
 	 * send them together in a single file-list. */
 	if (need_unsorted_flist) {
-		if (!(flist->sorted = new_array(struct file_struct *, flist->used)))
-			out_of_memory("send_file_list");
+		flist->sorted = new_array(struct file_struct *, flist->used);
 		memcpy(flist->sorted, flist->files,
 		       flist->used * sizeof (struct file_struct*));
 	} else
@@ -2597,8 +2589,7 @@ struct file_list *recv_file_list(int f, int dir_ndx)
 		 * order and for calling flist_find()).  We keep the "files"
 		 * list unsorted for our exchange of index numbers with the
 		 * other side (since their names may not sort the same). */
-		if (!(flist->sorted = new_array(struct file_struct *, flist->used)))
-			out_of_memory("recv_file_list");
+		flist->sorted = new_array(struct file_struct *, flist->used);
 		memcpy(flist->sorted, flist->files,
 		       flist->used * sizeof (struct file_struct*));
 		if (inc_recurse && dir_flist->used > dstart) {
@@ -2808,8 +2799,7 @@ struct file_list *flist_new(int flags, char *msg)
 {
 	struct file_list *flist;
 
-	if (!(flist = new0(struct file_list)))
-		out_of_memory(msg);
+	flist = new0(struct file_list);
 
 	if (flags & FLIST_TEMP) {
 		if (!(flist->file_pool = pool_create(SMALL_EXTENT, 0,

diff --git a/generator.c b/generator.c
@@ -2227,8 +2227,6 @@ void generate_files(int f_out, const char *local_name)
 	if (delete_during == 2) {
 		deldelay_size = BIGPATHBUFLEN * 4;
 		deldelay_buf = new_array(char, deldelay_size);
-		if (!deldelay_buf)
-			out_of_memory("delete-delay");
 	}
 	info_levels[INFO_FLIST] = info_levels[INFO_PROGRESS] = 0;
 

diff --git a/getgroups.c b/getgroups.c
@@ -20,8 +20,7 @@
 
 #include "rsync.h"
 
-int
-main(UNUSED(int argc), UNUSED(char *argv[]))
+ int main(UNUSED(int argc), UNUSED(char *argv[]))
 {
 	int n, i;
 	gid_t *list;

diff --git a/hashtable.c b/hashtable.c
@@ -35,9 +35,8 @@ struct hashtable *hashtable_create(int size, int key64)
 			size *= 2;
 	}
 
-	if (!(tbl = new(struct hashtable))
-	 || !(tbl->nodes = new_array0(char, size * node_size)))
-		out_of_memory("hashtable_create");
+	tbl = new(struct hashtable);
+	tbl->nodes = new_array0(char, size * node_size);
 	tbl->size = size;
 	tbl->entries = 0;
 	tbl->node_size = node_size;
@@ -94,8 +93,7 @@ void *hashtable_find(struct hashtable *tbl, int64 key, void *data_when_new)
 		int size = tbl->size * 2;
 		int i;
 
-		if (!(tbl->nodes = new_array0(char, size * tbl->node_size)))
-			out_of_memory("hashtable_node");
+		tbl->nodes = new_array0(char, size * tbl->node_size);
 		tbl->size = size;
 		tbl->entries = 0;