From: Omar Polo <op@omarpolo.com>
Subject: replace malloc+memcpy with strndup
To: gameoftrees@openbsd.org
Date: Tue, 10 Jan 2023 19:11:00 +0100

as per subject.  no functional change intended, just shorten some
chunks of code

ok?

diff e26970ccc755b7327924d761d1772a1bc5a5bf01 255bbe8320e32171cef392c166a99eb25ca32cab
commit - e26970ccc755b7327924d761d1772a1bc5a5bf01
commit + 255bbe8320e32171cef392c166a99eb25ca32cab
blob - 3c8cfad838be7bad650a556478fd84cdb32844f8
blob + 7f653f3ebb5d4da89d29058699af3f90cdb7baca
--- gotd/repo_write.c
+++ gotd/repo_write.c
@@ -349,11 +349,9 @@ recv_ref_update(struct imsg *imsg)
 
 	imsg_init(&ibuf, client->fd);
 
-	refname = malloc(iref.name_len + 1);
+	refname = strndup(imsg->data + sizeof(iref), iref.name_len);
 	if (refname == NULL)
-		return got_error_from_errno("malloc");
-	memcpy(refname, imsg->data + sizeof(iref), iref.name_len);
-	refname[iref.name_len] = '\0';
+		return got_error_from_errno("strndup");
 
 	ref_update = calloc(1, sizeof(*ref_update));
 	if (ref_update == NULL) {
blob - a6b8f011af70edd6ca8e1dd6ffc0658788c67ad1
blob + 4d1f43e554adcd668e7e8663eb9ffcb861dad3e6
--- gotd/session.c
+++ gotd/session.c
@@ -419,11 +419,9 @@ update_ref(struct gotd_session_client *client, const c
 	memcpy(&iref, imsg->data, sizeof(iref));
 	if (datalen != sizeof(iref) + iref.name_len)
 		return got_error(GOT_ERR_PRIVSEP_LEN);
-	refname = malloc(iref.name_len + 1);
+	refname = strndup(imsg->data + sizeof(iref), iref.name_len);
 	if (refname == NULL)
-		return got_error_from_errno("malloc");
-	memcpy(refname, imsg->data + sizeof(iref), iref.name_len);
-	refname[iref.name_len] = '\0';
+		return got_error_from_errno("strndup");
 
 	log_debug("updating ref %s for uid %d", refname, client->euid);
 
@@ -667,25 +665,18 @@ recv_capability(struct gotd_session_client *client, st
 	if (datalen != sizeof(icapa) + icapa.key_len + icapa.value_len)
 		return got_error(GOT_ERR_PRIVSEP_LEN);
 
-	key = malloc(icapa.key_len + 1);
+	key = strndp(imsg->data + sizeof(icapa), icapa.key_len);
 	if (key == NULL)
-		return got_error_from_errno("malloc");
+		return got_error_from_errno("strndup");
 	if (icapa.value_len > 0) {
-		value = malloc(icapa.value_len + 1);
+		value = strndup(imsg->data + sizeof(icapa) + icapa.key_len,
+		    icapa.value_len);
 		if (value == NULL) {
 			free(key);
-			return got_error_from_errno("malloc");
+			return got_error_from_errno("strndup");
 		}
 	}
 
-	memcpy(key, imsg->data + sizeof(icapa), icapa.key_len);
-	key[icapa.key_len] = '\0';
-	if (value) {
-		memcpy(value, imsg->data + sizeof(icapa) + icapa.key_len,
-		    icapa.value_len);
-		value[icapa.value_len] = '\0';
-	}
-
 	capa = &client->capabilities[client->ncapabilities++];
 	capa->key = key;
 	capa->value = value;
blob - a5c2c23ebfc28776e46ebd4b14022bf9b712e0e3
blob + 9762b99158cbd5c99d5b4169ddb59dc85d9f747c
--- lib/gitconfig.c
+++ lib/gitconfig.c
@@ -237,10 +237,9 @@ conf_parse_line(char **section, struct got_gitconfig *
 			*section = NULL;
 			return NULL;
 		}
-		*section = malloc(i);
+		*section = strndup(line + 1, i - 1);
 		if (*section == NULL)
-			return got_error_from_errno("malloc");
-		strlcpy(*section, line + 1, i);
+			return got_error_from_errno("strndup");
 		return NULL;
 	}
 	while (isspace((unsigned char)*line))
blob - 174d412f27d59a5180400ca4bbd33f6a4b8e5564
blob + d7a1dda565c2f2ee404c15276ebdd47e29965ed9
--- lib/privsep.c
+++ lib/privsep.c
@@ -2088,14 +2088,11 @@ got_privsep_recv_gitconfig_str(char **str, struct imsg
 	case GOT_IMSG_GITCONFIG_STR_VAL:
 		if (datalen == 0)
 			break;
-		/* datalen does not include terminating \0 */
-		*str = malloc(datalen + 1);
+		*str = strndup(imsg.data, datalen);
 		if (*str == NULL) {
-			err = got_error_from_errno("malloc");
+			err = got_error_from_errno("strndup");
 			break;
 		}
-		memcpy(*str, imsg.data, datalen);
-		(*str)[datalen] = '\0';
 		break;
 	default:
 		err = got_error(GOT_ERR_PRIVSEP_MSG);
@@ -2366,14 +2363,11 @@ got_privsep_recv_gotconfig_str(char **str, struct imsg
 	case GOT_IMSG_GOTCONFIG_STR_VAL:
 		if (datalen == 0)
 			break;
-		/* datalen does not include terminating \0 */
-		*str = malloc(datalen + 1);
+		*str = strndup(imsg.data, datalen);
 		if (*str == NULL) {
-			err = got_error_from_errno("malloc");
+			err = got_error_from_errno("strndup");
 			break;
 		}
-		memcpy(*str, imsg.data, datalen);
-		(*str)[datalen] = '\0';
 		break;
 	default:
 		err = got_error(GOT_ERR_PRIVSEP_MSG);
@@ -2828,9 +2822,9 @@ got_privsep_recv_enumerated_objects(int *found_all_obj
 			}
 			memcpy(tree_id.sha1, itree->id, sizeof(tree_id.sha1));
 			free(path);
-			path = malloc(path_len + 1);
+			path = strndup(imsg.data + sizeof(*itree), path_len);
 			if (path == NULL) {
-				err = got_error_from_errno("malloc");
+				err = got_error_from_errno("strndup");
 				break;
 			}
 			free(canon_path);
@@ -2839,9 +2833,6 @@ got_privsep_recv_enumerated_objects(int *found_all_obj
 				err = got_error_from_errno("malloc");
 				break;
 			}
-			memcpy(path, (uint8_t *)imsg.data + sizeof(*itree),
-			    path_len);
-			path[path_len] = '\0';
 			if (!got_path_is_absolute(path)) {
 				err = got_error(GOT_ERR_BAD_PATH);
 				break;
blob - 1916c9b0c7afe7a7fba9ce4f0009229fc986da6a
blob + af8f549fd73132008458becef0739c851b06c8be
--- lib/serve.c
+++ lib/serve.c
@@ -339,14 +339,12 @@ announce_refs(int outfd, struct imsgbuf *ibuf, int cli
 				err = got_error(GOT_ERR_PRIVSEP_LEN);
 				goto done;
 			}
-			refname = malloc(iref.name_len + 1);
-			if (refname == NULL) {
-				err = got_error_from_errno("malloc");
-				goto done;
-			}
-			memcpy(refname, imsg.data + sizeof(iref),
+			refname = strndup(imsg.data + sizeof(iref),
 			    iref.name_len);
-			refname[iref.name_len] = '\0';
+			if (refname == NULL) {
+				err = got_error_from_errno("strndup");
+				goto done;
+			}
 			err = send_ref(outfd, iref.id, refname,
 			    !sent_capabilities, client_is_reading,
 			    NULL, chattygot);
@@ -382,24 +380,20 @@ announce_refs(int outfd, struct imsgbuf *ibuf, int cli
 			    symrefname != NULL || symreftarget != NULL)
 				break;
 
-			symrefname = malloc(isymref.name_len + 1);
-			if (symrefname == NULL) {
-				err = got_error_from_errno("malloc");
-				goto done;
-			}
-			memcpy(symrefname, imsg.data + sizeof(isymref),
+			symrefname = strndup(imsg.data + sizeof(isymref),
 			    isymref.name_len);
-			symrefname[isymref.name_len] = '\0';
-
-			symreftarget = malloc(isymref.target_len + 1);
-			if (symreftarget == NULL) {
+			if (symrefname == NULL) {
 				err = got_error_from_errno("malloc");
 				goto done;
 			}
-			memcpy(symreftarget,
+
+			symreftarget = strndup(
 			    imsg.data + sizeof(isymref) + isymref.name_len,
 			    isymref.target_len);
-			symreftarget[isymref.target_len] = '\0';
+			if (symreftarget == NULL) {
+				err = got_error_from_errno("strndup");
+				goto done;
+			}
 
 			if (asprintf(&symrefstr, "%s:%s", symrefname,
 			    symreftarget) == -1) {
@@ -1219,13 +1213,11 @@ report_unpack_status(struct imsg *imsg, int outfd, int
 	if (datalen != sizeof(istatus) + istatus.reason_len)
 		return got_error(GOT_ERR_PRIVSEP_LEN);
 
-	reason = malloc(istatus.reason_len + 1);
+	reason = strndup(imsg->data + sizeof(istatus), istatus.reason_len);
 	if (reason == NULL) {
-		err = got_error_from_errno("malloc");
+		err = got_error_from_errno("strndup");
 		goto done;
 	}
-	memcpy(reason, imsg->data + sizeof(istatus), istatus.reason_len);
-	reason[istatus.reason_len] = '\0';
 
 	if (err == NULL)
 		len = snprintf(buf, sizeof(buf), "unpack ok\n");
@@ -1260,11 +1252,9 @@ recv_ref_update_ok(struct imsg *imsg, int outfd, int c
 
 	memcpy(&iok, imsg->data, sizeof(iok));
 
-	refname = malloc(iok.name_len + 1);
+	refname = strndup(imsg->data + sizeof(iok), iok.name_len);
 	if (refname == NULL)
-		return got_error_from_errno("malloc");
-	memcpy(refname, imsg->data + sizeof(iok), iok.name_len);
-	refname[iok.name_len] = '\0';
+		return got_error_from_errno("strndup");
 
 	len = snprintf(buf, sizeof(buf), "ok %s\n", refname);
 	if (len >= sizeof(buf)) {
@@ -1296,20 +1286,16 @@ recv_ref_update_ng(struct imsg *imsg, int outfd, int c
 
 	memcpy(&ing, imsg->data, sizeof(ing));
 
-	refname = malloc(ing.name_len + 1);
+	refname = strndup(imsg->data + sizeof(ing), ing.name_len);
 	if (refname == NULL)
-		return got_error_from_errno("malloc");
-	memcpy(refname, imsg->data + sizeof(ing), ing.name_len);
-	refname[ing.name_len] = '\0';
+		return got_error_from_errno("strndup");
 
-	reason = malloc(ing.reason_len + 1);
-	if (reason == NULL) {
-		err = got_error_from_errno("malloc");
-		goto done;
-	}
-	memcpy(refname, imsg->data + sizeof(ing) + ing.name_len,
+	reason = strndup(imsg->sizeof(ing) + ing.name_len,
 	    ing.reason_len);
-	refname[ing.reason_len] = '\0';
+	if (reason == NULL) {
+		err = got_error_from_errno("strndup");
+		goto done;
+	}
 
 	len = snprintf(buf, sizeof(buf), "ng %s %s\n", refname, reason);
 	if (len >= sizeof(buf)) {
blob - cbcfef7d6f9e5c10b699ada020cdf69f79490487
blob + 6e1ae2146ae101481c8223adead2d0fbc046ed00
--- libexec/got-fetch-pack/got-fetch-pack.c
+++ libexec/got-fetch-pack/got-fetch-pack.c
@@ -854,13 +854,11 @@ main(int argc, char **argv)
 			err = got_error(GOT_ERR_PRIVSEP_LEN);
 			goto done;
 		}
-		refname = malloc(href.name_len + 1);
+		refname = strndup(imsg.data + sizeof(href), href.name_len);
 		if (refname == NULL) {
-			err = got_error_from_errno("malloc");
+			err = got_error_from_errno("strndup");
 			goto done;
 		}
-		memcpy(refname, imsg.data + sizeof(href), href.name_len);
-		refname[href.name_len] = '\0';
 
 		id = malloc(sizeof(*id));
 		if (id == NULL) {
@@ -904,13 +902,12 @@ main(int argc, char **argv)
 			err = got_error(GOT_ERR_PRIVSEP_LEN);
 			goto done;
 		}
-		refname = malloc(wbranch.name_len + 1);
+		refname = strndup(imsg.data + sizeof(wbranch),
+		    wbranch.name_len);
 		if (refname == NULL) {
-			err = got_error_from_errno("malloc");
+			err = got_error_from_errno("strndup");
 			goto done;
 		}
-		memcpy(refname, imsg.data + sizeof(wbranch), wbranch.name_len);
-		refname[wbranch.name_len] = '\0';
 
 		err = got_pathlist_append(&wanted_branches, refname, NULL);
 		if (err) {
@@ -946,13 +943,11 @@ main(int argc, char **argv)
 			err = got_error(GOT_ERR_PRIVSEP_LEN);
 			goto done;
 		}
-		refname = malloc(wref.name_len + 1);
+		refname = strndup(imsg.data + sizeof(wref), wref.name_len);
 		if (refname == NULL) {
-			err = got_error_from_errno("malloc");
+			err = got_error_from_errno("strndup");
 			goto done;
 		}
-		memcpy(refname, imsg.data + sizeof(wref), wref.name_len);
-		refname[wref.name_len] = '\0';
 
 		err = got_pathlist_append(&wanted_refs, refname, NULL);
 		if (err) {