Re: [PATCH v4 05/12] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v4 05/12] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:08

Brandon Casey [off-list ref] writes:
quoted
quoted
+	return len > strlen(cherry_picked_prefix) + 1 &&
+		!prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
+}
Does the first "is it longer than the prefix?" check matter?  If it
is not, prefixcmp() would not match anyway, no?
Probably not in practice, but technically we should only be accessing
len characters in buf even though buf may be longer than len.  So the
check is just making sure the function doesn't access chars it's not
supposed to.
Sorry, I do not follow.  Isn't caller's buf terminated with LF at buf[len],
which would never match cherry_picked_prefix even if len is shorter
than the prefix?

Re: [PATCH v4 05/12] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer

From: Brandon Casey <hidden>
Date: 2016-06-15 22:56:08

On 2/12/2013 11:36 AM, Junio C Hamano wrote:
Brandon Casey [off-list ref] writes:
quoted
quoted
quoted
+	return len > strlen(cherry_picked_prefix) + 1 &&
+		!prefixcmp(buf, cherry_picked_prefix) && buf[len - 1] == ')';
+}
Does the first "is it longer than the prefix?" check matter?  If it
is not, prefixcmp() would not match anyway, no?
Probably not in practice, but technically we should only be accessing
len characters in buf even though buf may be longer than len.  So the
check is just making sure the function doesn't access chars it's not
supposed to.
Sorry, I do not follow.  Isn't caller's buf terminated with LF at buf[len],
which would never match cherry_picked_prefix even if len is shorter
than the prefix?
Heh, I almost pointed that out in my reply.  Yes, buf will be terminated
with LF at buf[len].  And yes, that means that we will never get a false
positive from prefixcmp even if the comparison overruns buf+len while
doing its comparison.  That's why the check doesn't matter in practice,
i.e. based on the way that is_cherry_picked_from_line is being called
right now and the content of cherry_picked_prefix.

But, hasn't is_cherry_picked_from_line entered into a contract with the
caller and said "I will not access more than len characters"?

It's ok with me if you think it reads better without the check.

-Brandon


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help