On Thu, Jul 11, 2024 at 03:02:18PM +0200, Omar Polo wrote:
> I need this in my tree, especially the second part, to not break
> everything horribly when extending the got_object_id struct.  The
> fileindex and worktree code (at least) are not yet ready for sha256, but
> this allows them to continue working, at least in my testing.
> 
> The first part is to not break gotd.  There are several places in gotd
> where we just copy stuff in/out the sha1 buffer, and those should also
> continue to work.  (sha256 in the network protocol is pretty down the
> todo list :/)
> 
> ok?
> 
> diff /home/op/w/got
> commit - 03a62f6dd737910257cd4602766adca863cc8811
> path + /home/op/w/got
> blob - 6c9c61a414a0d5c844ce86e60e9544b213cd637c
> file + gotd/session_write.c
> --- gotd/session_write.c
> +++ gotd/session_write.c
> @@ -696,7 +696,9 @@ update_ref(int *shut, struct gotd_session_client *clie
>  
>  	log_debug("updating ref %s for uid %d", refname, client->euid);
>  
> +	memset(&old_id, 0, sizeof(old_id));
>  	memcpy(old_id.sha1, iref.old_id, SHA1_DIGEST_LENGTH);
> +	memset(&new_id, 0, sizeof(new_id));
>  	memcpy(new_id.sha1, iref.new_id, SHA1_DIGEST_LENGTH);
>  	err = got_repo_find_object_id(iref.delete_ref ? &old_id : &new_id,
>  	    repo);

ok for the above gotd bits.

> blob - e4574472a88375bfac44ed049b2e5050ad77a389
> file + lib/object_qid.c
> --- lib/object_qid.c
> +++ lib/object_qid.c
> @@ -31,7 +31,7 @@
>  const struct got_error *
>  got_object_qid_alloc_partial(struct got_object_qid **qid)
>  {
> -	*qid = malloc(sizeof(**qid));
> +	*qid = calloc(1, sizeof(**qid));

This change defeats the purpose of "partial" allocation.
This function assumes callers will initialize other fields not
touched by this function. It's a hot-path micro-optimization.
I am not sure if it is still needed, it may come from a time
when parsed tree entries were a linked-list instead of an array. 

Would it be enough to keep malloc and set the hash algo field,
with a TODO comment saying callers should set the algo?

>  	if (*qid == NULL)
>  		return got_error_from_errno("malloc");

Would need to change to calloc here as well if calloc is the only
viable solution.