"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: struct vs buffer for got_imsg_{commit_,}object
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 1 Feb 2023 16:41:12 +0100

Download raw body.

Thread
On Wed, Feb 01, 2023 at 04:12:53PM +0100, Omar Polo wrote:
> separate diffs for easier reading (hopefully.)  the first one is
> trivial, the second one seems so but it's slightly tricky because in
> some structs the "tree_id" field is the struct in others the pointer
> to the struct: got_imsg_commit_object embeds the struct while
> got_commit_object dosen't.
> 
> ok?

ok

> commit 42c43229569244a64062c0b12fd11eef28120da8
> from: Omar Polo <op@omarpolo.com>
> date: Wed Feb  1 15:02:02 2023 UTC
>  
>  got_imsg_object: use struct instead of buffer for id
>  
> diff 38cd26a413a43dc0a7d5f80041fd705fe0d55773 42c43229569244a64062c0b12fd11eef28120da8
> commit - 38cd26a413a43dc0a7d5f80041fd705fe0d55773
> commit + 42c43229569244a64062c0b12fd11eef28120da8
> blob - 151e0d73fa443289c8609863654c6bef6d128723
> blob + 2817ac884ce25ec43a9812d10cca3e9dc017a4cf
> --- lib/got_lib_privsep.h
> +++ lib/got_lib_privsep.h
> @@ -220,7 +220,7 @@ struct got_imsg_object {
>   * Structure for GOT_IMSG_TREE_REQUEST and GOT_IMSG_OBJECT data.
>   */
>  struct got_imsg_object {
> -	uint8_t id[SHA1_DIGEST_LENGTH];
> +	struct got_object_id id;
>  
>  	/* These fields are the same as in struct got_object. */
>  	int type;
> blob - 970978560467422ed3a634b6c3b86e48e6b49fd0
> blob + 1f118154617c4dae7a5a12d5531b577d6d906da7
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -398,7 +398,7 @@ got_privsep_send_tree_req(struct imsgbuf *ibuf, int fd
>  	if (wbuf == NULL)
>  		return got_error_from_errno("imsg_create TREE_REQUEST");
>  
> -	if (imsg_add(wbuf, id->sha1, SHA1_DIGEST_LENGTH) == -1)
> +	if (imsg_add(wbuf, id, sizeof(*id)) == -1)
>  		return got_error_from_errno("imsg_add TREE_REQUEST");
>  
>  	if (pack_idx != -1) { /* tree is packed */
> @@ -510,7 +510,7 @@ got_privsep_send_obj(struct imsgbuf *ibuf, struct got_
>  
>  	memset(&iobj, 0, sizeof(iobj));
>  
> -	memcpy(iobj.id, obj->id.sha1, sizeof(iobj.id));
> +	memcpy(&iobj.id, &obj->id, sizeof(iobj.id));
>  	iobj.type = obj->type;
>  	iobj.flags = obj->flags;
>  	iobj.hdrlen = obj->hdrlen;
> @@ -1118,7 +1118,7 @@ got_privsep_get_imsg_obj(struct got_object **obj, stru
>  	if (*obj == NULL)
>  		return got_error_from_errno("calloc");
>  
> -	memcpy((*obj)->id.sha1, iobj->id, SHA1_DIGEST_LENGTH);
> +	memcpy(&(*obj)->id, &iobj->id, sizeof(iobj->id));
>  	(*obj)->type = iobj->type;
>  	(*obj)->flags = iobj->flags;
>  	(*obj)->hdrlen = iobj->hdrlen;
> 
> -----------------------------------------------
> commit 0c1022cbc2057b7964c07168452299432782b57b (main)
> from: Omar Polo <op@omarpolo.com>
> date: Wed Feb  1 15:02:02 2023 UTC
>  
>  got_imsg_commit_object: use struct instead of buffer for id
>  
> diff 42c43229569244a64062c0b12fd11eef28120da8 0c1022cbc2057b7964c07168452299432782b57b
> commit - 42c43229569244a64062c0b12fd11eef28120da8
> commit + 0c1022cbc2057b7964c07168452299432782b57b
> blob - 2817ac884ce25ec43a9812d10cca3e9dc017a4cf
> blob + 1b4b7bc8fc2baddfd1f3353725f35bd6ac9af0cf
> --- lib/got_lib_privsep.h
> +++ lib/got_lib_privsep.h
> @@ -233,7 +233,7 @@ struct got_imsg_commit_object {
>  
>  /* Structure for GOT_IMSG_COMMIT data. */
>  struct got_imsg_commit_object {
> -	uint8_t tree_id[SHA1_DIGEST_LENGTH];
> +	struct got_object_id tree_id;
>  	size_t author_len;
>  	time_t author_time;
>  	time_t author_gmtoff;
> @@ -247,7 +247,7 @@ struct got_imsg_commit_object {
>  	 * Followed by author_len + committer_len data bytes
>  	 */
>  
> -	/* Followed by 'nparents' SHA1_DIGEST_LENGTH length strings */
> +	/* Followed by 'nparents' struct got_object_id */
>  
>  	/*
>  	 * Followed by 'logmsg_len' bytes of commit log message data in
> blob - 1f118154617c4dae7a5a12d5531b577d6d906da7
> blob + d40d65ea1e93b234a672d9edd3fef1c550898939
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -1203,15 +1203,14 @@ got_privsep_send_commit(struct imsgbuf *ibuf, struct g
>  	size_t logmsg_len = strlen(commit->logmsg);
>  
>  	total = sizeof(*icommit) + author_len + committer_len +
> -	    commit->nparents * SHA1_DIGEST_LENGTH;
> +	    commit->nparents * sizeof(struct got_object_id);
>  
>  	buf = malloc(total);
>  	if (buf == NULL)
>  		return got_error_from_errno("malloc");
>  
>  	icommit = (struct got_imsg_commit_object *)buf;
> -	memcpy(icommit->tree_id, commit->tree_id->sha1,
> -	    sizeof(icommit->tree_id));
> +	memcpy(&icommit->tree_id, commit->tree_id, sizeof(icommit->tree_id));
>  	icommit->author_len = author_len;
>  	icommit->author_time = commit->author_time;
>  	icommit->author_gmtoff = commit->author_gmtoff;
> @@ -1227,8 +1226,8 @@ got_privsep_send_commit(struct imsgbuf *ibuf, struct g
>  	memcpy(buf + len, commit->committer, committer_len);
>  	len += committer_len;
>  	STAILQ_FOREACH(qid, &commit->parent_ids, entry) {
> -		memcpy(buf + len, &qid->id, SHA1_DIGEST_LENGTH);
> -		len += SHA1_DIGEST_LENGTH;
> +		memcpy(buf + len, &qid->id, sizeof(qid->id));
> +		len += sizeof(qid->id);
>  	}
>  
>  	if (imsg_compose(ibuf, GOT_IMSG_COMMIT, 0, 0, -1, buf, len) == -1) {
> @@ -1263,7 +1262,7 @@ get_commit_from_imsg(struct got_commit_object **commit
>  	icommit = imsg->data;
>  	if (datalen != sizeof(*icommit) + icommit->author_len +
>  	    icommit->committer_len +
> -	    icommit->nparents * SHA1_DIGEST_LENGTH)
> +	    icommit->nparents * sizeof(struct got_object_id))
>  		return got_error(GOT_ERR_PRIVSEP_LEN);
>  
>  	if (icommit->nparents < 0)
> @@ -1276,8 +1275,8 @@ get_commit_from_imsg(struct got_commit_object **commit
>  		return got_error_from_errno(
>  		    "got_object_commit_alloc_partial");
>  
> -	memcpy((*commit)->tree_id->sha1, icommit->tree_id,
> -	    SHA1_DIGEST_LENGTH);
> +	memcpy((*commit)->tree_id, &icommit->tree_id,
> +	    sizeof(icommit->tree_id));
>  	(*commit)->author_time = icommit->author_time;
>  	(*commit)->author_gmtoff = icommit->author_gmtoff;
>  	(*commit)->committer_time = icommit->committer_time;
> @@ -1342,7 +1341,7 @@ get_commit_from_imsg(struct got_commit_object **commit
>  		if (err)
>  			break;
>  		memcpy(&qid->id, imsg->data + len +
> -		    i * SHA1_DIGEST_LENGTH, sizeof(qid->id));
> +		    i * sizeof(qid->id), sizeof(qid->id));
>  		STAILQ_INSERT_TAIL(&(*commit)->parent_ids, qid, entry);
>  		(*commit)->nparents++;
>  	}
> 
>