From: Stefan Sperling Subject: Re: fileindex: add functions to fill object ids To: Omar Polo Cc: gameoftrees@openbsd.org Date: Thu, 9 Feb 2023 18:12:12 +0100 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); > >