Hi, brian m. carlson wrote: > Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> > --- > walker.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) An egrep for 'sha1|unsigned char' finds nothing left in the file after this change. [...] > --- a/walker.c > +++ b/walker.c > @@ -7,7 +7,7 @@ > #include "blob.h" > #include "refs.h" > > -static unsigned char current_commit_sha1[20]; > +static struct object_id current_commit_oid; Yay! nit, not suggesting changing anything: I would have been tempted to call this current_commit_id, or even just current_commmit. [...] > @@ -259,7 +259,7 @@ int walker_fetch(struct walker *walker, int targets, char **target, > struct strbuf refname = STRBUF_INIT; > struct strbuf err = STRBUF_INIT; > struct ref_transaction *transaction = NULL; > - unsigned char *sha1 = xmalloc(targets * 20); > + struct object_id *oids = xmalloc(targets * sizeof(struct object_id)); Not new in this patch, just noticing while we're here: can this multiplication overflow? E.g. in remote-curl, it looks like nr_heads gets incremented once per "fetch" line passed to the remote helper, making it unbounded. This could use st_mult or ALLOC_ARRAY to protect against that. The caller remote-curl.c::parse_fetch also needs an overflow check to avoid overflowing its "int". Likewise, walker_targets_stdin needs an overflow check to avoid overflowing its "int". [...] > @@ -321,7 +321,7 @@ int walker_fetch(struct walker *walker, int targets, char **target, > done: > ref_transaction_free(transaction); > free(msg); > - free(sha1); > + free(oids); Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
