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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: provide functions to parse/serialize different hashes
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 23 Feb 2023 13:39:21 +0100

Download raw body.

Thread
On Thu, Feb 23, 2023 at 11:48:27AM +0100, Omar Polo wrote:
> I've partially misunderstood your suggestion.  Here's a better diff.
> it completely retires got_parse_sha1() in favour of
> got_parse_hash_digest, but keeps got_sha1_digest_to_str().  Will look
> later how to drop that too in favour of a generic got_hash_digest_to_str().

Indeed, what you've done here is what I had in mind.
This could have easily been done as a follow-up change, too, I didn't
mean to block you on this.

Still ok by me.

> diffstat /home/op/w/got
>  M  include/got_object.h                     |   5+  0-
>  M  lib/got_lib_hash.h                       |   7+  1-
>  M  lib/hash.c                               |  54+  9-
>  M  lib/object.c                             |   1+  1-
>  M  lib/object_parse.c                       |   5+  3-
>  M  lib/patch.c                              |   1+  1-
>  M  lib/reference.c                          |   4+  2-
>  M  lib/repository.c                         |   4+  2-
>  M  lib/repository_admin.c                   |   3+  3-
>  M  lib/serve.c                              |   5+  4-
>  M  libexec/got-fetch-pack/got-fetch-pack.c  |   3+  2-
>  M  libexec/got-read-patch/got-read-patch.c  |   1+  1-
>  M  libexec/got-send-pack/got-send-pack.c    |   1+  1-
>  M  regress/idset/idset_test.c               |   3+  3-
> 
> 14 files changed, 97 insertions(+), 33 deletions(-)
> 
> diff /home/op/w/got
> commit - 53bf0b541977b66862040d4b633fb6b5d3a3c6c8
> path + /home/op/w/got
> blob - 6a161bc20df5aaa7789395593ab2ffc6fbe4b1fb
> file + include/got_object.h
> --- include/got_object.h
> +++ include/got_object.h
> @@ -16,6 +16,11 @@ struct got_object_id {
>  
>  #define GOT_OBJECT_ID_HEX_MAXLEN SHA1_DIGEST_STRING_LENGTH
>  
> +enum got_hash_algorithm {
> +	GOT_HASH_SHA1,
> +	GOT_HASH_SHA256,
> +};
> +
>  struct got_object_id {
>  	u_int8_t sha1[SHA1_DIGEST_LENGTH];
>  };
> blob - 141cb2590fa14834c63381929a0a7438ae3e4556
> file + lib/got_lib_hash.h
> --- lib/got_lib_hash.h
> +++ lib/got_lib_hash.h
> @@ -15,7 +15,13 @@
>   */
>  
>  #define GOT_SHA1_STRING_ZERO "0000000000000000000000000000000000000000"
> +#define GOT_SHA256_STRING_ZERO "0000000000000000000000000000000000000000000000000000000000000000"
>  
>  int got_parse_xdigit(uint8_t *, const char *);
> -int got_parse_sha1_digest(uint8_t *, const char *);
> +
>  char *got_sha1_digest_to_str(const uint8_t *, char *, size_t);
> +char *got_sha256_digest_to_str(const uint8_t *, char *, size_t);
> +
> +int got_parse_hash_digest(uint8_t *, const char *, enum got_hash_algorithm);
> +int got_parse_object_id(struct got_object_id *, const char *,
> +    enum got_hash_algorithm);
> blob - fb4d3a30adf9281507aea3424f4c2711b1d3a28c
> file + lib/hash.c
> --- lib/hash.c
> +++ lib/hash.c
> @@ -15,13 +15,18 @@
>   */
>  
>  #include <sys/types.h>
> +#include <sys/queue.h>
> +
>  #include <sha1.h>
>  #include <sha2.h>
>  #include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <limits.h>
>  
> +#include "got_object.h"
> +
>  #include "got_lib_hash.h"
>  
>  int
> @@ -41,14 +46,14 @@ int
>  	return 1;
>  }
>  
> -int
> -got_parse_sha1_digest(uint8_t *digest, const char *line)
> +static int
> +parse_digest(uint8_t *digest, int len, const char *line)
>  {
>  	uint8_t b = 0;
>  	char hex[3] = {'\0', '\0', '\0'};
>  	int i, j;
>  
> -	for (i = 0; i < SHA1_DIGEST_LENGTH; i++) {
> +	for (i = 0; i < len; i++) {
>  		if (line[0] == '\0' || line[1] == '\0')
>  			return 0;
>  		for (j = 0; j < 2; j++) {
> @@ -63,17 +68,14 @@ char *
>  	return 1;
>  }
>  
> -char *
> -got_sha1_digest_to_str(const uint8_t *digest, char *buf, size_t size)
> +static char *
> +digest_to_str(const uint8_t *digest, int len, char *buf)
>  {
>  	char *p = buf;
>  	char hex[3];
>  	int i;
>  
> -	if (size < SHA1_DIGEST_STRING_LENGTH)
> -		return NULL;
> -
> -	for (i = 0; i < SHA1_DIGEST_LENGTH; i++) {
> +	for (i = 0; i < len; i++) {
>  		snprintf(hex, sizeof(hex), "%.2x", digest[i]);
>  		p[0] = hex[0];
>  		p[1] = hex[1];
> @@ -83,3 +85,46 @@ got_sha1_digest_to_str(const uint8_t *digest, char *bu
>  
>  	return buf;
>  }
> +
> +char *
> +got_sha1_digest_to_str(const uint8_t *digest, char *buf, size_t size)
> +{
> +	if (size < SHA1_DIGEST_STRING_LENGTH)
> +		return NULL;
> +	return digest_to_str(digest, SHA1_DIGEST_LENGTH, buf);
> +}
> +
> +char *
> +got_sha256_digest_to_str(const uint8_t *digest, char *buf, size_t size)
> +{
> +	if (size < SHA256_DIGEST_STRING_LENGTH)
> +		return NULL;
> +	return digest_to_str(digest, SHA256_DIGEST_LENGTH, buf);
> +}
> +
> +int
> +got_parse_hash_digest(uint8_t *digest, const char *line,
> +    enum got_hash_algorithm algo)
> +{
> +	switch (algo) {
> +	case GOT_HASH_SHA1:
> +		return parse_digest(digest, SHA1_DIGEST_LENGTH, line);
> +	case GOT_HASH_SHA256:
> +		return parse_digest(digest, SHA256_DIGEST_LENGTH, line);
> +	default:
> +		return 0;
> +	}
> +}
> +
> +int
> +got_parse_object_id(struct got_object_id *id, const char *line,
> +    enum got_hash_algorithm algo)
> +{
> +	memset(id, 0, sizeof(*id));
> +
> +	/* XXX: temporary until we grow got_object_id */
> +	if (algo != GOT_HASH_SHA1)
> +		return 0;
> +
> +	return got_parse_hash_digest(id->sha1, line, algo);
> +}
> blob - f8b902a4c27338568c455eb639ecba5cbb38b8c2
> file + lib/object.c
> --- lib/object.c
> +++ lib/object.c
> @@ -151,7 +151,7 @@ got_object_open_by_id_str(struct got_object **obj, str
>  {
>  	struct got_object_id id;
>  
> -	if (!got_parse_sha1_digest(id.sha1, id_str))
> +	if (!got_parse_object_id(&id, id_str, GOT_HASH_SHA1))
>  		return got_error_path(id_str, GOT_ERR_BAD_OBJ_ID_STR);
>  
>  	return got_object_open(obj, repo, &id);
> blob - 67c3bc37ea78571d6f3c25478ae3e612ee235f22
> file + lib/object_parse.c
> --- lib/object_parse.c
> +++ lib/object_parse.c
> @@ -387,7 +387,7 @@ got_object_commit_add_parent(struct got_commit_object 
>  	if (err)
>  		return err;
>  
> -	if (!got_parse_sha1_digest(qid->id.sha1, id_str)) {
> +	if (!got_parse_object_id(&qid->id, id_str, GOT_HASH_SHA1)) {
>  		err = got_error(GOT_ERR_BAD_OBJ_DATA);
>  		got_object_qid_free(qid);
>  		return err;
> @@ -620,6 +620,7 @@ got_object_parse_commit(struct got_commit_object **com
>      size_t len)
>  {
>  	const struct got_error *err = NULL;
> +	enum got_hash_algorithm algo = GOT_HASH_SHA1;
>  	char *s = buf;
>  	size_t label_len;
>  	ssize_t remain = (ssize_t)len;
> @@ -639,7 +640,7 @@ got_object_parse_commit(struct got_commit_object **com
>  			goto done;
>  		}
>  		s += label_len;
> -		if (!got_parse_sha1_digest((*commit)->tree_id->sha1, s)) {
> +		if (!got_parse_object_id((*commit)->tree_id, s, algo)) {
>  			err = got_error(GOT_ERR_BAD_OBJ_DATA);
>  			goto done;
>  		}
> @@ -974,6 +975,7 @@ got_object_parse_tag(struct got_tag_object **tag, uint
>  got_object_parse_tag(struct got_tag_object **tag, uint8_t *buf, size_t len)
>  {
>  	const struct got_error *err = NULL;
> +	enum got_hash_algorithm algo = GOT_HASH_SHA1;
>  	size_t remain = len;
>  	char *s = buf;
>  	size_t label_len;
> @@ -993,7 +995,7 @@ got_object_parse_tag(struct got_tag_object **tag, uint
>  			goto done;
>  		}
>  		s += label_len;
> -		if (!got_parse_sha1_digest((*tag)->id.sha1, s)) {
> +		if (!got_parse_object_id(&(*tag)->id, s, algo)) {
>  			err = got_error(GOT_ERR_BAD_OBJ_DATA);
>  			goto done;
>  		}
> blob - 92514335bde2a392e0211cb8f5903c931aac55d2
> file + lib/patch.c
> --- lib/patch.c
> +++ lib/patch.c
> @@ -706,7 +706,7 @@ open_blob(char **path, FILE **fp, const char *blobid,
>  			return err;
>  		idptr = matched_id;
>  	} else {
> -		if (!got_parse_sha1_digest(id.sha1, blobid))
> +		if (!got_parse_object_id(&id, blobid, GOT_HASH_SHA1))
>  			return got_error(GOT_ERR_BAD_OBJ_ID_STR);
>  		idptr = &id;
>  	}
> blob - b7e19bcfc7a34cea70cf48a5c32ab66ee4acb857
> file + lib/reference.c
> --- lib/reference.c
> +++ lib/reference.c
> @@ -155,6 +155,7 @@ parse_ref_line(struct got_reference **ref, const char 
>  parse_ref_line(struct got_reference **ref, const char *name, const char *line,
>      time_t mtime)
>  {
> +	enum got_hash_algorithm algo = GOT_HASH_SHA1;
>  	struct got_object_id id;
>  
>  	if (strncmp(line, "ref: ", 5) == 0) {
> @@ -162,7 +163,7 @@ parse_ref_line(struct got_reference **ref, const char 
>  		return parse_symref(ref, name, line);
>  	}
>  
> -	if (!got_parse_sha1_digest(id.sha1, line))
> +	if (!got_parse_object_id(&id, line, algo))
>  		return got_error(GOT_ERR_BAD_REF_DATA);
>  
>  	return alloc_ref(ref, name, &id, 0, mtime);
> @@ -292,6 +293,7 @@ parse_packed_ref_line(struct got_reference **ref, cons
>  parse_packed_ref_line(struct got_reference **ref, const char *abs_refname,
>      const char *line, time_t mtime)
>  {
> +	enum got_hash_algorithm algo = GOT_HASH_SHA1;
>  	struct got_object_id id;
>  	const char *name;
>  
> @@ -300,7 +302,7 @@ parse_packed_ref_line(struct got_reference **ref, cons
>  	if (line[0] == '#' || line[0] == '^')
>  		return NULL;
>  
> -	if (!got_parse_sha1_digest(id.sha1, line))
> +	if (!got_parse_object_id(&id, line, algo))
>  		return got_error(GOT_ERR_BAD_REF_DATA);
>  
>  	if (abs_refname) {
> blob - abff91e2c234cdd1c87f05d07d2a9d02a0cc98e4
> file + lib/repository.c
> --- lib/repository.c
> +++ lib/repository.c
> @@ -1738,6 +1738,7 @@ match_loose_object(struct got_object_id **unique_id, c
>  	}
>  	while ((dent = readdir(dir)) != NULL) {
>  		int cmp;
> +		enum got_hash_algorithm algo = GOT_HASH_SHA1;
>  
>  		free(id_str);
>  		id_str = NULL;
> @@ -1751,7 +1752,7 @@ match_loose_object(struct got_object_id **unique_id, c
>  			goto done;
>  		}
>  
> -		if (!got_parse_sha1_digest(id.sha1, id_str))
> +		if (!got_parse_object_id(&id, id_str, algo))
>  			continue;
>  
>  		/*
> @@ -2291,6 +2292,7 @@ got_repo_get_loose_object_info(int *nobjects, off_t *o
>  			char *id_str;
>  			int fd;
>  			struct stat sb;
> +			enum got_hash_algorithm algo = GOT_HASH_SHA1;
>  
>  			if (strcmp(dent->d_name, ".") == 0 ||
>  			    strcmp(dent->d_name, "..") == 0)
> @@ -2301,7 +2303,7 @@ got_repo_get_loose_object_info(int *nobjects, off_t *o
>  				goto done;
>  			}
>  
> -			if (!got_parse_sha1_digest(id.sha1, id_str)) {
> +			if (!got_parse_object_id(&id, id_str, algo)) {
>  				free(id_str);
>  				continue;
>  			}
> blob - d552b4a35a689baa8cf628c010eb5adfc9d57ce4
> file + lib/repository_admin.c
> --- lib/repository_admin.c
> +++ lib/repository_admin.c
> @@ -486,7 +486,7 @@ got_repo_find_pack(FILE **packfile, struct got_object_
>  		goto done;
>  	}
>  	*dot = '\0';
> -	if (!got_parse_sha1_digest(id.sha1, p)) {
> +	if (!got_parse_object_id(&id, p, GOT_HASH_SHA1)) {
>  		err = got_error_fmt(GOT_ERR_BAD_PATH,
>  		    "'%s' is not a valid pack file name",
>  		    packfile_name);
> @@ -685,8 +685,8 @@ get_loose_object_ids(struct got_object_idset **loose_i
>  				goto done;
>  			}
>  
> -			memset(&id, 0, sizeof(id));
> -			if (!got_parse_sha1_digest(id.sha1, id_str)) {
> +			if (!got_parse_object_id(&id, id_str,
> +			    GOT_HASH_SHA1)) {
>  				free(id_str);
>  				continue;
>  			}
> blob - ecd71cee9f0fb1d68f5a2341e98bfa386fb743c0
> file + lib/serve.c
> --- lib/serve.c
> +++ lib/serve.c
> @@ -36,6 +36,7 @@
>  #include "got_path.h"
>  #include "got_version.h"
>  #include "got_reference.h"
> +#include "got_object.h"
>  
>  #include "got_lib_pkt.h"
>  #include "got_lib_dial.h"
> @@ -433,7 +434,7 @@ parse_want_line(char **common_capabilities, uint8_t *i
>  	if (err)
>  		return err;
>  
> -	if (!got_parse_sha1_digest(id, id_str)) {
> +	if (!got_parse_hash_digest(id, id_str, GOT_HASH_SHA1)) {
>  		err = got_error_msg(GOT_ERR_BAD_PACKET,
>  		    "want-line with bad object ID");
>  		goto done;
> @@ -462,7 +463,7 @@ parse_have_line(uint8_t *id, char *buf, size_t len)
>  	if (err)
>  		return err;
>  
> -	if (!got_parse_sha1_digest(id, id_str)) {
> +	if (!got_parse_hash_digest(id, id_str, GOT_HASH_SHA1)) {
>  		err = got_error_msg(GOT_ERR_BAD_PACKET,
>  		    "have-line with bad object ID");
>  		goto done;
> @@ -1049,8 +1050,8 @@ parse_ref_update_line(char **common_capabilities, char
>  	if (err)
>  		return err;
>  
> -	if (!got_parse_sha1_digest(old_id, old_id_str) ||
> -	    !got_parse_sha1_digest(new_id, new_id_str)) {
> +	if (!got_parse_hash_digest(old_id, old_id_str, GOT_HASH_SHA1) ||
> +	    !got_parse_hash_digest(new_id, new_id_str, GOT_HASH_SHA1)) {
>  		err = got_error_msg(GOT_ERR_BAD_PACKET,
>  		    "ref-update with bad object ID");
>  		goto done;
> blob - 9182db0e5c9f34de75291565e8162a16cda84db4
> file + libexec/got-fetch-pack/got-fetch-pack.c
> --- libexec/got-fetch-pack/got-fetch-pack.c
> +++ libexec/got-fetch-pack/got-fetch-pack.c
> @@ -300,7 +300,7 @@ fetch_ref(struct imsgbuf *ibuf, struct got_pathlist_he
>  	const struct got_error *err;
>  	char *theirs = NULL, *mine = NULL;
>  
> -	if (!got_parse_sha1_digest(want->sha1, id_str)) {
> +	if (!got_parse_object_id(want, id_str, GOT_HASH_SHA1)) {
>  		err = got_error(GOT_ERR_BAD_OBJ_ID_STR);
>  		goto done;
>  	}
> @@ -619,7 +619,8 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
>  			    "unexpected message from server");
>  			goto done;
>  		}
> -		if (!got_parse_sha1_digest(common_id.sha1, buf + 4)) {
> +		if (!got_parse_object_id(&common_id, buf + 4,
> +		    GOT_HASH_SHA1)) {
>  			err = got_error_msg(GOT_ERR_BAD_PACKET,
>  			    "bad object ID in ACK packet from server");
>  			goto done;
> blob - 4849e5e3e5d7c3c90f43a4b6a30f2e39b23f2820
> file + libexec/got-read-patch/got-read-patch.c
> --- libexec/got-read-patch/got-read-patch.c
> +++ libexec/got-read-patch/got-read-patch.c
> @@ -193,7 +193,7 @@ blobid(const char *line, char **blob, int git)
>  	if ((*blob = strndup(line, len)) == NULL)
>  		return got_error_from_errno("strndup");
>  
> -	if (!git && !got_parse_sha1_digest(digest, *blob)) {
> +	if (!git && !got_parse_hash_digest(digest, *blob, GOT_HASH_SHA1)) {
>  		/* silently ignore invalid blob ids */
>  		free(*blob);
>  		*blob = NULL;
> blob - d1ffc719f8ca4925cb7bf8c3d4e5c8180824fb64
> file + libexec/got-send-pack/got-send-pack.c
> --- libexec/got-send-pack/got-send-pack.c
> +++ libexec/got-send-pack/got-send-pack.c
> @@ -414,7 +414,7 @@ send_pack(int fd, struct got_pathlist_head *refs,
>  			err = got_error_from_errno("malloc");
>  			goto done;
>  		}
> -		if (!got_parse_sha1_digest(id->sha1, id_str)) {
> +		if (!got_parse_object_id(id, id_str, GOT_HASH_SHA1)) {
>  			err = got_error(GOT_ERR_BAD_OBJ_ID_STR);
>  			goto done;
>  		}
> blob - 511fd016b578a9308aede2f46618c2b7b6a353c2
> file + regress/idset/idset_test.c
> --- regress/idset/idset_test.c
> +++ regress/idset/idset_test.c
> @@ -70,15 +70,15 @@ idset_add_remove_iter(void)
>  		goto done;
>  	}
>  
> -	if (!got_parse_sha1_digest(id1.sha1, id_str1)) {
> +	if (!got_parse_object_id(&id1, id_str1, GOT_HASH_SHA1)) {
>  		err = got_error(GOT_ERR_BAD_OBJ_ID_STR);
>  		goto done;
>  	}
> -	if (!got_parse_sha1_digest(id2.sha1, id_str2)) {
> +	if (!got_parse_object_id(&id2, id_str2, GOT_HASH_SHA1)) {
>  		err = got_error(GOT_ERR_BAD_OBJ_ID_STR);
>  		goto done;
>  	}
> -	if (!got_parse_sha1_digest(id3.sha1, id_str3)) {
> +	if (!got_parse_object_id(&id3, id_str3, GOT_HASH_SHA1)) {
>  		err = got_error(GOT_ERR_BAD_OBJ_ID_STR);
>  		goto done;
>  	}
>