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.