| Message ID | cb8dde2ae2e78d0bcfb61382fe7769c12804336b.1588275891.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | shallow: extract a header file | expand |
Taylor Blau <me@ttaylorr.com> writes: > -static int commit_graft_pos(struct repository *r, const unsigned char *sha1) > +int commit_graft_pos(struct repository *r, const unsigned char *sha1) > { > return sha1_pos(sha1, r->parsed_objects->grafts, > r->parsed_objects->grafts_nr, > diff --git a/commit.h b/commit.h > index ab91d21131..eb42e8b6d2 100644 > --- a/commit.h > +++ b/commit.h > @@ -236,6 +236,7 @@ struct commit_graft { > typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *); > > struct commit_graft *read_graft_line(struct strbuf *line); > +int commit_graft_pos(struct repository *r, const unsigned char *sha1); In an earlier exchange, I saw this: >> - could include a comment saying that it's an index into >> r->parsed_objects->grafts > > This and the below are both good ideas to me. I prefer this one, since > we'd have to duplicate yet another static function > ('commit_graft_sha1_access()' directly above) that is called by this > one. > >> - I'm usually loathe to suggest unnecessary duplication of code, but >> it might make sense to duplicate the function into shallow.c. Or >> even to inline it there (in the single call site, that ends up >> being pretty readable). > > I am not at all offended by duplication of code where it makes sense to > do so, but having to duplicate two functions seems like we'd be better > off simply documenting the function in commit.h. and I think I agree with that direction. Forgot to add those comments?
On Thu, Apr 30, 2020 at 01:55:11PM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > > -static int commit_graft_pos(struct repository *r, const unsigned char *sha1) > > +int commit_graft_pos(struct repository *r, const unsigned char *sha1) > > { > > return sha1_pos(sha1, r->parsed_objects->grafts, > > r->parsed_objects->grafts_nr, > > diff --git a/commit.h b/commit.h > > index ab91d21131..eb42e8b6d2 100644 > > --- a/commit.h > > +++ b/commit.h > > @@ -236,6 +236,7 @@ struct commit_graft { > > typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *); > > > > struct commit_graft *read_graft_line(struct strbuf *line); > > +int commit_graft_pos(struct repository *r, const unsigned char *sha1); > > In an earlier exchange, I saw this: > > >> - could include a comment saying that it's an index into > >> r->parsed_objects->grafts > > > > This and the below are both good ideas to me. I prefer this one, since > > we'd have to duplicate yet another static function > > ('commit_graft_sha1_access()' directly above) that is called by this > > one. > > > >> - I'm usually loathe to suggest unnecessary duplication of code, but > >> it might make sense to duplicate the function into shallow.c. Or > >> even to inline it there (in the single call site, that ends up > >> being pretty readable). > > > > I am not at all offended by duplication of code where it makes sense to > > do so, but having to duplicate two functions seems like we'd be better > > off simply documenting the function in commit.h. > > and I think I agree with that direction. Forgot to add those > comments? Thanks for remembering. I did apply this locally, but must've dropped it on the floor during a rebase. In any case, please swap in this patch instead: -- >8 -- Subject: [PATCH] commit: make 'commit_graft_pos' non-static In the next patch, some functions will be moved from 'commit.c' to have prototypes in a new 'shallow.h' and their implementations in 'shallow.c'. Three functions in 'commit.c' use 'commit_graft_pos()' (they are 'register_commit_graft()', 'lookup_commit_graft()', and 'unregister_shallow()'). The first two of these will stay in 'commit.c', but the latter will move to 'shallow.c', and thus needs 'commit_graft_pos' to be non-static. Prepare for that by making 'commit_graft_pos' non-static so that it can be called from both 'commit.c' and 'shallow.c'. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- commit.c | 2 +- commit.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/commit.c b/commit.c index c7099daeac..b76f7d72be 100644 --- a/commit.c +++ b/commit.c @@ -110,7 +110,7 @@ static const unsigned char *commit_graft_sha1_access(size_t index, void *table) return commit_graft_table[index]->oid.hash; } -static int commit_graft_pos(struct repository *r, const unsigned char *sha1) +int commit_graft_pos(struct repository *r, const unsigned char *sha1) { return sha1_pos(sha1, r->parsed_objects->grafts, r->parsed_objects->grafts_nr, diff --git a/commit.h b/commit.h index ab91d21131..0fe1e1b570 100644 --- a/commit.h +++ b/commit.h @@ -236,6 +236,8 @@ struct commit_graft { typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *); struct commit_graft *read_graft_line(struct strbuf *line); +/* commit_graft_pos returns an index into r->parsed_objects->grafts. */ +int commit_graft_pos(struct repository *r, const unsigned char *sha1); int register_commit_graft(struct repository *r, struct commit_graft *, int); void prepare_commit_graft(struct repository *r); struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid); -- 2.26.0.113.ge9739cdccc
diff --git a/commit.c b/commit.c index c7099daeac..b76f7d72be 100644 --- a/commit.c +++ b/commit.c @@ -110,7 +110,7 @@ static const unsigned char *commit_graft_sha1_access(size_t index, void *table) return commit_graft_table[index]->oid.hash; } -static int commit_graft_pos(struct repository *r, const unsigned char *sha1) +int commit_graft_pos(struct repository *r, const unsigned char *sha1) { return sha1_pos(sha1, r->parsed_objects->grafts, r->parsed_objects->grafts_nr, diff --git a/commit.h b/commit.h index ab91d21131..eb42e8b6d2 100644 --- a/commit.h +++ b/commit.h @@ -236,6 +236,7 @@ struct commit_graft { typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *); struct commit_graft *read_graft_line(struct strbuf *line); +int commit_graft_pos(struct repository *r, const unsigned char *sha1); int register_commit_graft(struct repository *r, struct commit_graft *, int); void prepare_commit_graft(struct repository *r); struct commit_graft *lookup_commit_graft(struct repository *r, const struct object_id *oid);
In the next patch, some functions will be moved from 'commit.c' to have prototypes in a new 'shallow.h' and their implementations in 'shallow.c'. Three functions in 'commit.c' use 'commit_graft_pos()' (they are 'register_commit_graft()', 'lookup_commit_graft()', and 'unregister_shallow()'). The first two of these will stay in 'commit.c', but the latter will move to 'shallow.c', and thus needs 'commit_graft_pos' to be non-static. Prepare for that by making 'commit_graft_pos' non-static so that it can be called from both 'commit.c' and 'shallow.c'. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- commit.c | 2 +- commit.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)