| Message ID | f256d2058736091c3d9662788762849e8df794b0.1580413221.git.matheus.bernardino@usp.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix inconsistent uses of the_repo in parse_object()'s call chain | expand |
On Thu, Jan 30, 2020 at 05:32:21PM -0300, Matheus Tavares wrote: > Allow write_object_file_prepare() to receive arbitrary 'struct > git_hash_algo's instead of always using the_hash_algo. The added > parameter will be used in the next commit to make hash_object_file() be > able to work with arbitrary git_hash_algo's, as well. > > Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> > --- > sha1-file.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/sha1-file.c b/sha1-file.c > index 13b90b1cb1..e789bfd153 100644 > --- a/sha1-file.c > +++ b/sha1-file.c > @@ -1647,7 +1647,8 @@ void *read_object_with_reference(struct repository *r, > } > } > > -static void write_object_file_prepare(const void *buf, unsigned long len, > +static void write_object_file_prepare(const struct git_hash_algo *algo, > + const void *buf, unsigned long len, > const char *type, struct object_id *oid, > char *hdr, int *hdrlen) > { One minor minor suggestion/nit, may be my own type of style only. When adding a parameter to a function, we typically add it at the end of the parameter list, rather than at the beginning. The other (unwritten) convention, when dealing with "buffer pointer/len", seams to be that buffer-ptr is the first paramter and len is is the second. However, this is my personal note, other opinions and comments are welcome. What do others think ?
On Sat, Feb 01, 2020 at 11:03:37AM +0100, Torsten Bögershausen wrote: > > diff --git a/sha1-file.c b/sha1-file.c > > index 13b90b1cb1..e789bfd153 100644 > > --- a/sha1-file.c > > +++ b/sha1-file.c > > @@ -1647,7 +1647,8 @@ void *read_object_with_reference(struct repository *r, > > } > > } > > > > -static void write_object_file_prepare(const void *buf, unsigned long len, > > +static void write_object_file_prepare(const struct git_hash_algo *algo, > > + const void *buf, unsigned long len, > > const char *type, struct object_id *oid, > > char *hdr, int *hdrlen) > > { > > One minor minor suggestion/nit, may be my own type of style only. > > When adding a parameter to a function, we typically add it at the end of > the parameter list, rather than at the beginning. > The other (unwritten) convention, when dealing with "buffer pointer/len", > seams to be that buffer-ptr is the first paramter and len is is the second. I'd usually add new options at the end, too, all things being equal. But in this case, I see: - some of these are input parameters and some are output (I think oid, hdr, and hdrlen are the latter); it makes sense to keep them grouped that way - in object-oriented functions, we'd often take the object first (e.g., strbuf_add). In non-object-oriented functions, we often take a "context" argument first that shapes how the function works (e.g., all of the pp_* functions that take a pretty_print_context). I'd think of the algorithm a little bit as a "context" argument, though I admit it's mostly a gut feeling and quite subjective. But if you believe that line of reasoning, then it doesn't seem out of place as the first argument. -Peff
diff --git a/sha1-file.c b/sha1-file.c index 13b90b1cb1..e789bfd153 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -1647,7 +1647,8 @@ void *read_object_with_reference(struct repository *r, } } -static void write_object_file_prepare(const void *buf, unsigned long len, +static void write_object_file_prepare(const struct git_hash_algo *algo, + const void *buf, unsigned long len, const char *type, struct object_id *oid, char *hdr, int *hdrlen) { @@ -1657,10 +1658,10 @@ static void write_object_file_prepare(const void *buf, unsigned long len, *hdrlen = xsnprintf(hdr, *hdrlen, "%s %"PRIuMAX , type, (uintmax_t)len)+1; /* Sha1.. */ - the_hash_algo->init_fn(&c); - the_hash_algo->update_fn(&c, hdr, *hdrlen); - the_hash_algo->update_fn(&c, buf, len); - the_hash_algo->final_fn(oid->hash, &c); + algo->init_fn(&c); + algo->update_fn(&c, hdr, *hdrlen); + algo->update_fn(&c, buf, len); + algo->final_fn(oid->hash, &c); } /* @@ -1718,7 +1719,8 @@ int hash_object_file(const void *buf, unsigned long len, const char *type, { char hdr[MAX_HEADER_LEN]; int hdrlen = sizeof(hdr); - write_object_file_prepare(buf, len, type, oid, hdr, &hdrlen); + write_object_file_prepare(the_hash_algo, buf, len, type, oid, hdr, + &hdrlen); return 0; } @@ -1876,7 +1878,8 @@ int write_object_file(const void *buf, unsigned long len, const char *type, /* Normally if we have it in the pack then we do not bother writing * it out into .git/objects/??/?{38} file. */ - write_object_file_prepare(buf, len, type, oid, hdr, &hdrlen); + write_object_file_prepare(the_hash_algo, buf, len, type, oid, hdr, + &hdrlen); if (freshen_packed_object(oid) || freshen_loose_object(oid)) return 0; return write_loose_object(oid, hdr, hdrlen, buf, len, 0); @@ -1892,7 +1895,8 @@ int hash_object_file_literally(const void *buf, unsigned long len, /* type string, SP, %lu of the length plus NUL must fit this */ hdrlen = strlen(type) + MAX_HEADER_LEN; header = xmalloc(hdrlen); - write_object_file_prepare(buf, len, type, oid, header, &hdrlen); + write_object_file_prepare(the_hash_algo, buf, len, type, oid, header, + &hdrlen); if (!(flags & HASH_WRITE_OBJECT)) goto cleanup;
Allow write_object_file_prepare() to receive arbitrary 'struct git_hash_algo's instead of always using the_hash_algo. The added parameter will be used in the next commit to make hash_object_file() be able to work with arbitrary git_hash_algo's, as well. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> --- sha1-file.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)