From: Stefan Sperling Subject: Re: provide functions to parse/serialize different hashes To: Omar Polo Cc: gameoftrees@openbsd.org Date: Thu, 23 Feb 2023 13:39:21 +0100 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 > +#include > + > #include > #include > #include > #include > #include > +#include > #include > > +#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; > } >