Re: [PATCH 5/5] refspec.c: use rhs in parse_refspec instead of potentially uninitialized item->dst

Junio C Hamano [off-list ref] writes:

Perhaps a better fisx is to explicitly assign NULL to item->dst when
we see there is no right-hand-side.

-- >8 --
Subject: [PATCH] refspec-api: avoid uninitialized field in refspec item

When parse_refspec() function was created at 3eec3700 ("refspec:
factor out parsing a single refspec", 2018-05-16) to take a caller
supplied piece of memory to fill parsed refspec_item, it forgot that
a refspec without colon must set item->dst to NULL to let the users
of refspec know that the result of the fetch does not get stored in
an ref on our side.

Signed-off-by: Junio C Hamano <redacted>
---

 * The original before that change filled a callee prepared piece of
   memory that was obtained from xcalloc(), and did not need to
   explicitly assign NULL to the field after noticing that there is
   no colon in the refspec, so it is understandable how this
   misconvesion happened.

 refspec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/refspec.c b/refspec.c
index 97e76e8b1d..6e45365a23 100644
--- a/refspec.c
+++ b/refspec.c

@@ -48,6 +48,8 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet
 		size_t rlen = strlen(++rhs);
 		is_glob = (1 <= rlen && strchr(rhs, '*'));
 		item->dst = xstrndup(rhs, rlen);
+	} else {
+		item->dst = NULL;
 	}
 
 	llen = (rhs ? (rhs - lhs - 1) : strlen(lhs));

-- 
2.18.0-rc0

On Thu, May 31, 2018 at 7:46 PM, Junio C Hamano [off-list ref] wrote:

Junio C Hamano [off-list ref] writes:

quoted

Perhaps a better fisx is to explicitly assign NULL to item->dst when
we see there is no right-hand-side.

-- >8 --
Subject: [PATCH] refspec-api: avoid uninitialized field in refspec item

When parse_refspec() function was created at 3eec3700 ("refspec:
factor out parsing a single refspec", 2018-05-16) to take a caller
supplied piece of memory to fill parsed refspec_item, it forgot that
a refspec without colon must set item->dst to NULL to let the users
of refspec know that the result of the fetch does not get stored in
an ref on our side.

Signed-off-by: Junio C Hamano <redacted>

This looks correct. Thanks for writing the patch.

Did I miss the other 4 patches that this might depend on it?

No. I was sloppy and developed a couple of patches on top
of pu and then tried to put them onto their respective branches.
and then I forgot to correct the patch counts after figuring out the right
branch to apply this to.

Thanks,
Stefan