If the repository contained an expanded ident keyword (i.e. $Id:XXXX$),
then the wrong bytes were discarded, and the Id keyword was not
expanded. The fault was in convert.c:ident_to_worktree().
Previously, when a "$Id:" was found in the repository version,
ident_to_worktree() would search for the next "$" after this, and
discarded everything it found until then. That was done with the loop:
do {
ch = *cp++;
if (ch == '$')
break;
rem--;
} while (rem);
The above loop left cp pointing one character _after_ the final "$"
(because of ch = *cp++). This was different from the non-expanded case,
were cp is left pointing at the "$", and was different from the comment
which stated "discard up to but not including the closing $". This
patch fixes that by making the loop:
do {
ch = *cp;
if (ch == '$')
break;
cp++;
rem--;
} while (rem);
That is, cp is tested _then_ incremented.
This loop exits if it finds a "$" or if it runs out of bytes in the
source. After this loop, if there was no closing "$" the expansion is
skipped, and the outer loop is allowed to continue leaving this
non-keyword as it was. However, when the "$" is found, size is
corrected, before running the expansion:
size -= (cp - src);
This is wrong; size is going to be corrected anyway after the expansion,
so there is no need to do it here. This patch removes that redundant
correction.
To help find this bug, I heavily commented the routine; those comments
are included here as a bonus.
Signed-off-by: Andy Parkins <andyparkins@xxxxxxxxx>
---
You wouldn't believe that I managed to get a file into the repository
with $Id$ stored expanded would you? :-)
Anyway, it's fortunate that I did, because it revealed the above bugs
in the ident_to_worktree() code.
I've included the comments I wrote while debugging in this patch, which
I'm sure will annoy you, because you'd rather the fix and the comments
separately. I'll supply that if you wish - just holler.
convert.c | 39 +++++++++++++++++++++++++++++++++++++--
1 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/convert.c b/convert.c
index 4b26b1a..3c44e3d 100644
--- a/convert.c
+++ b/convert.c
@@ -509,36 +509,71 @@ static char *ident_to_worktree(const char *path, const char *src, unsigned long
for (dst = buf; size; size--) {
const char *cp;
+ /* Fetch next source character, move the pointer on */
char ch = *src++;
+ /* Copy the current character to the destination */
*dst++ = ch;
+ /* If the current character is "$" or there are less than three
+ * remaining bytes or the two bytes following this one are not
+ * "Id", then simply read the next character */
if ((ch != '$') || (size < 3) || memcmp("Id", src, 2))
continue;
+ /*
+ * Here when
+ * - There are more than 2 bytes remaining
+ * - The current three bytes are "$Id$"
+ * with
+ * - ch == "$"
+ * - src[0] == "I"
+ */
+ /*
+ * It's possible that an expanded Id has crept its way into the
+ * repository, we cope with that by stripping the expansion out
+ */
if (src[2] == ':') {
+ /* Expanded keywords have "$Id:" at the front */
+
/* discard up to but not including the closing $ */
unsigned long rem = size - 3;
+ /* Point at first byte after the ":" */
cp = src + 3;
+ /*
+ * Throw away characters until either
+ * - we reach a "$"
+ * - we run out of bytes (rem == 0)
+ */
do {
- ch = *cp++;
+ ch = *cp;
if (ch == '$')
break;
+ cp++;
rem--;
} while (rem);
+ /* If the above finished because it ran out of characters, then
+ * this is an incomplete keyword, so don't run the expansion */
if (!rem)
continue;
- size -= (cp - src);
} else if (src[2] == '$')
cp = src + 2;
else
+ /* Anything other than "$Id:XXX$" or $Id$ and we skip the
+ * expansion */
continue;
+ /* cp is now pointing at the last $ of the keyword */
+
memcpy(dst, "Id: ", 4);
dst += 4;
memcpy(dst, sha1_to_hex(sha1), 40);
dst += 40;
*dst++ = ' ';
+
+ /* Adjust for the characters we've discarded */
size -= (cp - src);
src = cp;
+
+ /* Copy the final "$" */
*dst++ = *src++;
size--;
}
--
1.5.2.763.g8c5e-dirty
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
