Re: [PATCH v2 11/53] fast-import: convert to struct object_id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 01, 2017 at 03:07:22PM -0700, Jonathan Tan wrote:

> > @@ -2298,8 +2296,12 @@ static uintmax_t do_change_note_fanout(
> >  static uintmax_t change_note_fanout(struct tree_entry *root,
> >  		unsigned char fanout)
> >  {
> > -	char hex_sha1[40], path[60];
> > -	return do_change_note_fanout(root, root, hex_sha1, 0, path, 0, fanout);
> > +	/*
> > +	 * The size of path is due to one slash between every two hex digits,
> > +	 * plus the terminating NUL.
> > +	 */
> > +	char hex_oid[GIT_MAX_HEXSZ], path[GIT_MAX_HEXSZ * 3 / 2];
> 
> If your comment is correct, shouldn't the size of path be 61 (that is, add
> "+ 1")? I took a look at do_change_note_fanout() and your comment seems
> correct.

If you have 40 hex digits, then you have 20 hex pairs. But delimiting
them all takes only 19 slashes, since they only go in between pairs[1].

So the fully expanded formula is:

  GIT_MAX_HEXSZ +               (1) actual hex bytes
  (GIT_MAX_HEXSZ / 2) - 1 +     (2) internal slashes between pairs
  1                             (3) NUL terminator

which simplifies to 3/2 GIT_MAX_HEXSZ. It may be better to write it out
(the compiler can simplify) or explain that in the comment, though. It
took me a minute to figure out that it was correct, too.

-Peff

[1] This is sort of a reverse-fencepost error:
    https://en.wikipedia.org/wiki/Off-by-one_error#Fencepost_error



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]