On Thu, Feb 23, 2023 at 10:26:35AM +0100, Omar Polo wrote:
> Hello,
> 
> This adds an alternative set of functions to deal with sha256 hash
> digest that behaves like the sha1 ones we already have.  On top of
> these there's a generic got_parse_hash_digest() that uses the hash
> algorithm specified as argument.  Finally, to prevent future breakage,
> this adds got_parse_object_id() that fills the struct got_object_id.
> The memset() is important: in the future I'd like to grow it with a
> field, and the explicit zeroing will prevent breakages due to
> un-initialized fields (we have many object ids lying on the stack.)
> 
> Finally, I'm also replacing almost all got_parse_sha1_digest() calls,
> only a few in gotd and one in got-read-patch remains.  The various
> enum got_hash_algorithm introduced in various places will be soon
> bubbled up and disappear soon-ish.
> 
> ok?

Yes, looks good to me.

> @@ -83,3 +88,73 @@ got_sha1_digest_to_str(const uint8_t *digest, char *bu
>  
>  	return buf;
>  }
> +
> +int
> +got_parse_sha256_digest(uint8_t *digest, const char *line)
> +{
> +	uint8_t b = 0;
> +	char hex[3] = {'\0', '\0', '\0'};
> +	int i, j;

The implementations of this and the SHA1 version are identical except
for the length, correct? In whihc case we could have a common function,
internal to hash.c, which takes a length argument.

> +	for (i = 0; i < SHA256_DIGEST_LENGTH; i++) {
> +		if (line[0] == '\0' || line[1] == '\0')
> +			return 0;
> +		for (j = 0; j < 2; j++) {
> +			hex[j] = *line;
> +			line++;
> +		}
> +		if (!got_parse_xdigit(&b, hex))
> +			return 0;
> +		digest[i] = b;
> +	}
> +
> +	return 1;
> +}
> +
> +char *
> +got_sha256_digest_to_str(const uint8_t *digest, char *buf, size_t size)
> +{
> +	char *p = buf;
> +	char hex[3];
> +	int i;

Same here?

> +
> +	if (size < SHA256_DIGEST_STRING_LENGTH)
> +		return NULL;
> +
> +	for (i = 0; i < SHA256_DIGEST_LENGTH; i++) {
> +		snprintf(hex, sizeof(hex), "%.2x", digest[i]);
> +		p[0] = hex[0];
> +		p[1] = hex[1];
> +		p += 2;
> +	}
> +	p[0] = '\0';
> +
> +	return buf;
> +}
> +
> +int
> +got_parse_hash_digest(uint8_t *digest, const char *line,
> +    enum got_hash_algorithm algo)
> +{
> +	switch (algo) {
> +	case GOT_HASH_SHA1:
> +		return got_parse_sha1_digest(digest, line);

The length would be passed here.

> +	case GOT_HASH_SHA256:
> +		return got_parse_sha256_digest(digest, line);

And here.