From: Stefan Sperling Subject: Re: introduce got_object_id_serialize To: Omar Polo Cc: gameoftrees@openbsd.org Date: Fri, 3 Feb 2023 12:44:04 +0100 On Fri, Feb 03, 2023 at 11:52:20AM +0100, Omar Polo wrote: > add a function that's analogous to got_object_id_str but writes to the > given buffer. at the moment is just a wrapper around got_sha1_digest_to_str > but will gain some more logic in the future. > > i've avoided all the calls to got_sha1_digest_to_str that were in the > pack code, the network code and gotd. There are a few calls to build > a GOT_ERR_OBJ_CSUM error that i'm going to deal with in a follow-up > diff. The two calls in the patch code were kept too, will revisit > them when we'll start to have some real initial sha256 support. > > diffstat -s /home/op/w/got > M include/got_object.h | 7+ 0- Does the function need to be declared in exactly this headers? Would lib/got_lib_object.h work, too? This would reduce visibility. Code outside lib libexec (and now, gotd) isn't really supposed to be using headers from lib/. For new functions it would be better to start out with limited visibility if possible. > M lib/error.c | 1+ 1- > M lib/object.c | 1+ 1- > M lib/object_parse.c | 7+ 1- > > 4 files changed, 16 insertions(+), 3 deletions(-) > > diff -s /home/op/w/got > commit - 09cbb981df84d9c12f0fa371cb6b85d3f1b615c0 > path + /home/op/w/got (staged changes) > blob - 9d11c8f4f518f5f911295671d3aafcb494a7ec10 > blob + 3dea2df3656c85f425a01b9e66471da1fc10fdaf > --- include/got_object.h > +++ include/got_object.h > @@ -82,6 +82,13 @@ const struct got_error *got_object_id_str(char **, str > const struct got_error *got_object_id_str(char **, struct got_object_id *); > > /* > + * Write the string representation of an object ID in the given buffer. > + * The output depends on the hash function used by the repository format > + * (currently SHA1). > + */ > +char *got_object_id_serialize(struct got_object_id *, char *, size_t); "serialize" sounds as if it was creating a bytes string representation. I would suggest using a different term. For example: got_object_id_to_hex(struct got_object_id *, char *, size_t); There should be a constant which callers could use to create a buffer of an appropriate size. For now, we could define it like: #define GOT_OBJECT_ID_HEX_MAXLEN SHA1_DIGEST_LENGTH and adjust this constant later as needed. The docs could say: * Write the string representation of an object ID in the given buffer. * This buffer must be at least GOT_OBJECT_ID_HEX_MAXLEN bytes in size. * The output depends on the hash function used by the repository format. And I would drop (currently SHA1). We don't need to document this because callers shouldn't have to care.