On 2023/01/10 15:05:07 -0500, Bryan Steele <brynet@gmail.com> wrote:
> On Tue, Jan 10, 2023 at 07:11:00PM +0100, Omar Polo wrote:
> > as per subject.  no functional change intended, just shorten some
> > chunks of code
> > 
> > ok?
> > 
> > ...
> > @@ -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);
> 
> Typo here, but you probably already noticed. :-)

No idea how that ended up there.  there was another typo in the diff,
sorry.  Probably i've forgot to rebuild gotd after the first sweep.

updated and recheked diff, this one should be fine :)

thanks!

-----------------------------------------------
commit 5055132302bb46b787a31fd6a2b46d50fb2eb431 (x)
from: Omar Polo <op@omarpolo.com>
date: Tue Jan 10 22:07:30 2023 UTC
 
 replace malloc+memcpy with strndup.  no functional change intended
 
diff e26970ccc755b7327924d761d1772a1bc5a5bf01 5055132302bb46b787a31fd6a2b46d50fb2eb431
commit - e26970ccc755b7327924d761d1772a1bc5a5bf01
commit + 5055132302bb46b787a31fd6a2b46d50fb2eb431
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 + c40b69f8f26a58591321c159a47de3d0835b9dfb
--- 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 = strndup(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 + 6e4c3ef0627fe92f92f4091b2926040aa3d9d9db
--- 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->data + 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) {