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 05/01/2017 03:27 PM, Jeff King wrote:
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

Ah...yes, you're right. (And I checked that do_change_note_fanout() does place the slashes only between characters, not before or after them.)


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]