Re: [PATCH v2 01/24] walker: convert to struct object_id
- Date: Mon, 9 Oct 2017 15:37:09 -0700
- From: Jonathan Nieder <jrnieder@xxxxxxxxx>
- Subject: Re: [PATCH v2 01/24] walker: convert to struct object_id
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
> --- a/walker.c
> +++ b/walker.c
> @@ -7,7 +7,7 @@
> #include "blob.h"
> #include "refs.h"
> -static unsigned char current_commit_sha1;
> +static struct object_id current_commit_oid;
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
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,
> - free(sha1);
> + free(oids);
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>