"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: fileindex: add functions to fill object ids
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 9 Feb 2023 18:12:12 +0100

Download raw body.

Thread
On Tue, Feb 07, 2023 at 03:56:19PM +0100, Omar Polo wrote:
> This adds a few functions to fill a struct got_object_id from specific
> fields of a got fileindex entry.  In worktree.c we have a few calls
> like
> 
> 	struct got_object_id id;
> 	memcpy(id.sha1, ie->blob_sha1, SHA1_DIGEST_LENGTH);
> 
> that will break when the struct got_object_id will grow more fields.
> The hardcoded hash size is not a problem for the moment, will still
> take a while to reach the point of checking out worktrees from sha256
> repos.
> 
> The proposed got_fileindex_entry_get_*_id set of functions will
> instead first zero the id, and as not planned consequence leads to
> slightly shorted code in worktree.c (albeit the loooong function
> names.)
> 
> ok?

Yes, seems sane. Ok.

> diff /home/op/w/got
> commit - 188f8dcf2c1c15bf37859e3b587bc6331fd5a097
> path + /home/op/w/got
> blob - 6d94e2ef4017332aaa8d60289a09e393f1dbbd99
> file + lib/fileindex.c
> --- lib/fileindex.c
> +++ lib/fileindex.c
> @@ -1230,4 +1230,31 @@ RB_GENERATE(got_fileindex_tree, got_fileindex_entry, e
>  	return err;
>  }
>  
> +struct got_object_id *
> +got_fileindex_entry_get_staged_blob_id(struct got_object_id *id,
> +    struct got_fileindex_entry *ie)
> +{
> +	memset(id, 0, sizeof(*id));
> +	memcpy(id->sha1, ie->staged_blob_sha1, sizeof(ie->staged_blob_sha1));
> +	return id;
> +}
> +
> +struct got_object_id *
> +got_fileindex_entry_get_blob_id(struct got_object_id *id,
> +    struct got_fileindex_entry *ie)
> +{
> +	memset(id, 0, sizeof(*id));
> +	memcpy(id->sha1, ie->blob_sha1, sizeof(ie->blob_sha1));
> +	return id;
> +}
> +
> +struct got_object_id *
> +got_fileindex_entry_get_commit_id(struct got_object_id *id,
> +    struct got_fileindex_entry *ie)
> +{
> +	memset(id, 0, sizeof(*id));
> +	memcpy(id->sha1, ie->commit_sha1, sizeof(ie->commit_sha1));
> +	return id;
> +}
> +
>  RB_GENERATE(got_fileindex_tree, got_fileindex_entry, entry, got_fileindex_cmp);
> blob - 0c22385de5665de7e4366e6db2b42d3737058e0f
> file + lib/got_lib_fileindex.h
> --- lib/got_lib_fileindex.h
> +++ lib/got_lib_fileindex.h
> @@ -174,3 +174,14 @@ void got_fileindex_entry_mark_deleted_from_disk(struct
>  int got_fileindex_entry_staged_filetype_get(struct got_fileindex_entry *);
>  
>  void got_fileindex_entry_mark_deleted_from_disk(struct got_fileindex_entry *);
> +
> +/*
> + * Retrieve staged, blob or commit id from a fileindex entry, and return
> + * the given object id.
> + */
> +struct got_object_id *got_fileindex_entry_get_staged_blob_id(
> +    struct got_object_id *, struct got_fileindex_entry *);
> +struct got_object_id *got_fileindex_entry_get_blob_id(struct got_object_id *,
> +    struct got_fileindex_entry *);
> +struct got_object_id *got_fileindex_entry_get_commit_id(struct got_object_id *,
> +    struct got_fileindex_entry *);
> blob - 68a09786e6a0e800e79ea54e221f936dea9da837
> file + lib/worktree.c
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -1717,9 +1717,9 @@ get_file_status(unsigned char *status, struct stat *sb
>  
>  	if (staged_status == GOT_STATUS_MODIFY ||
>  	    staged_status == GOT_STATUS_ADD)
> -		memcpy(id.sha1, ie->staged_blob_sha1, sizeof(id.sha1));
> +		got_fileindex_entry_get_staged_blob_id(&id, ie);
>  	else
> -		memcpy(id.sha1, ie->blob_sha1, sizeof(id.sha1));
> +		got_fileindex_entry_get_blob_id(&id, ie);
>  
>  	fd1 = got_opentempfd();
>  	if (fd1 == -1) {
> @@ -1929,7 +1929,7 @@ update_blob(struct got_worktree *worktree,
>  				goto done;
>  			}
>  			struct got_object_id id2;
> -			memcpy(id2.sha1, ie->blob_sha1, SHA1_DIGEST_LENGTH);
> +			got_fileindex_entry_get_blob_id(&id2, ie);
>  			err = got_object_open_as_blob(&blob2, repo, &id2, 8192,
>  			    fd2);
>  			if (err)
> @@ -3343,19 +3343,14 @@ report_file_status(struct got_fileindex_entry *ie, con
>  	    staged_status == GOT_STATUS_NO_CHANGE && !report_unchanged)
>  		return NULL;
>  
> -	if (got_fileindex_entry_has_blob(ie)) {
> -		memcpy(blob_id.sha1, ie->blob_sha1, SHA1_DIGEST_LENGTH);
> -		blob_idp = &blob_id;
> -	}
> -	if (got_fileindex_entry_has_commit(ie)) {
> -		memcpy(commit_id.sha1, ie->commit_sha1, SHA1_DIGEST_LENGTH);
> -		commit_idp = &commit_id;
> -	}
> +	if (got_fileindex_entry_has_blob(ie))
> +		blob_idp = got_fileindex_entry_get_blob_id(&blob_id, ie);
> +	if (got_fileindex_entry_has_commit(ie))
> +		commit_idp = got_fileindex_entry_get_commit_id(&commit_id, ie);
>  	if (staged_status == GOT_STATUS_ADD ||
>  	    staged_status == GOT_STATUS_MODIFY) {
> -		memcpy(staged_blob_id.sha1, ie->staged_blob_sha1,
> -		    SHA1_DIGEST_LENGTH);
> -		staged_blob_idp = &staged_blob_id;
> +		staged_blob_idp = got_fileindex_entry_get_staged_blob_id(
> +		    &staged_blob_id, ie);
>  	}
>  
>  	return (*status_cb)(status_arg, status, staged_status,
> @@ -3407,8 +3402,8 @@ status_old(void *arg, struct got_fileindex_entry *ie, 
>  	if (!got_path_is_child(ie->path, a->status_path, a->status_path_len))
>  		return NULL;
>  
> -	memcpy(blob_id.sha1, ie->blob_sha1, SHA1_DIGEST_LENGTH);
> -	memcpy(commit_id.sha1, ie->commit_sha1, SHA1_DIGEST_LENGTH);
> +	got_fileindex_entry_get_blob_id(&blob_id, ie);
> +	got_fileindex_entry_get_commit_id(&commit_id, ie);
>  	if (got_fileindex_entry_has_file_on_disk(ie))
>  		status = GOT_STATUS_MISSING;
>  	else
> @@ -4782,12 +4777,10 @@ revert_file(void *arg, unsigned char status, unsigned 
>  	case GOT_STATUS_MISSING: {
>  		struct got_object_id id;
>  		if (staged_status == GOT_STATUS_ADD ||
> -		    staged_status == GOT_STATUS_MODIFY) {
> -			memcpy(id.sha1, ie->staged_blob_sha1,
> -			    SHA1_DIGEST_LENGTH);
> -		} else
> -			memcpy(id.sha1, ie->blob_sha1,
> -			    SHA1_DIGEST_LENGTH);
> +		    staged_status == GOT_STATUS_MODIFY)
> +			got_fileindex_entry_get_staged_blob_id(&id, ie);
> +		else
> +			got_fileindex_entry_get_blob_id(&id, ie);
>  		fd = got_opentempfd();
>  		if (fd == -1) {
>  			err = got_error_from_errno("got_opentempfd");
> @@ -8282,9 +8275,8 @@ check_stage_ok(void *arg, unsigned char status,
>  		return got_error_from_errno("asprintf");
>  
>  	if (got_fileindex_entry_has_commit(ie)) {
> -		memcpy(base_commit_id.sha1, ie->commit_sha1,
> -		    SHA1_DIGEST_LENGTH);
> -		base_commit_idp = &base_commit_id;
> +		base_commit_idp = got_fileindex_entry_get_commit_id(
> +		    &base_commit_id, ie);
>  	}
>  
>  	if (status == GOT_STATUS_CONFLICT) {
> @@ -9116,22 +9108,17 @@ report_file_info(void *arg, struct got_fileindex_entry
>  	if (pe == NULL) /* not found */
>  		return NULL;
>  
> -	if (got_fileindex_entry_has_blob(ie)) {
> -		memcpy(blob_id.sha1, ie->blob_sha1, SHA1_DIGEST_LENGTH);
> -		blob_idp = &blob_id;
> -	}
> +	if (got_fileindex_entry_has_blob(ie))
> +		blob_idp = got_fileindex_entry_get_blob_id(&blob_id, ie);
>  	stage = got_fileindex_entry_stage_get(ie);
>  	if (stage == GOT_FILEIDX_STAGE_MODIFY ||
>  	    stage == GOT_FILEIDX_STAGE_ADD) {
> -		memcpy(staged_blob_id.sha1, ie->staged_blob_sha1,
> -		    SHA1_DIGEST_LENGTH);
> -		staged_blob_idp = &staged_blob_id;
> +		staged_blob_idp = got_fileindex_entry_get_staged_blob_id(
> +		    &staged_blob_id, ie);
>  	}
>  
> -	if (got_fileindex_entry_has_commit(ie)) {
> -		memcpy(commit_id.sha1, ie->commit_sha1, SHA1_DIGEST_LENGTH);
> -		commit_idp = &commit_id;
> -	}
> +	if (got_fileindex_entry_has_commit(ie))
> +		commit_idp = got_fileindex_entry_get_commit_id(&commit_id, ie);
>  
>  	return a->info_cb(a->info_arg, ie->path, got_fileindex_perms_to_st(ie),
>  	    (time_t)ie->mtime_sec, blob_idp, staged_blob_idp, commit_idp);
> 
>