Re: [PATCH 05/38] pack v4: add commit object parsing

> Hi,
> 
> 
> On Thu, Sep 05, 2013 at 02:19:28AM -0400, Nicolas Pitre wrote:
> > Let's create another dictionary table to hold the author and committer
> > entries.  We use the same table format used for tree entries where the
> > 16 bit data prefix is conveniently used to store the timezone value.
> > 
> > In order to copy straight from a commit object buffer, dict_add_entry()
> > is modified to get the string length as the provided string pointer is
> > not always be null terminated.
> > 
> > Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
> > ---
> >  packv4-create.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 89 insertions(+), 9 deletions(-)
> > 
> > diff --git a/packv4-create.c b/packv4-create.c
> > index eccd9fc..5c08871 100644
> > --- a/packv4-create.c
> > +++ b/packv4-create.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * packv4-create.c: management of dictionary tables used in pack v4
> > + * packv4-create.c: creation of dictionary tables and objects used in pack v4
> >   *
> >   * (C) Nicolas Pitre <nico@fluxnic.net>
> >   *
> > @@ -80,9 +80,9 @@ static void rehash_entries(struct dict_table *t)
> >  	}
> >  }
> >  
> > -int dict_add_entry(struct dict_table *t, int val, const char *str)
> > +int dict_add_entry(struct dict_table *t, int val, const char *str, int str_len)
> >  {
> > -	int i, val_len = 2, str_len = strlen(str) + 1;
> > +	int i, val_len = 2;
> >  
> >  	if (t->ptr + val_len + str_len > t->size) {
> 
> We need a +1 here on the left side, i.e.
> 
>         if (t->ptr + val_len + str_len + 1 > t->size) {

> Sidenote: couldn't we call the 'ptr' field something else, like
> end_offset or end_idx?  It took me some headscratching to figure out
> why is it OK to compare a pointer to an integer above, or use a
> pointer without dereferencing as an index into an array below (because
> ptr is, well, not a pointer after all).