On Tuesday 2007 October 02, Jeff King wrote:
> - patlen != namelen - 5 &&
> - prefixcmp(name, "refs/heads/") &&
> - prefixcmp(name, "refs/tags/")) {
> + patlen != namelen - STRLEN_PATH_REFS_HEADS &&
> + prefixcmp(name, PATH_REFS_HEADS) &&
> + prefixcmp(name, PATH_REFS_HEADS)) {
>
> This is totally bogus. You meant STRLEN_PATH_REFS, and the second path
> should be PATH_REFS_TAGS. With those changes, t5516 passes.
Excellent! Well done. I spent a couple of hours last night going through
every changed line and have spotted the TAGS mistake but didn't spot the
STRLEN being wrong. Amazing how easy it is to become blind to these things.
There were a couple of errors in "/" placement too, but I don't think they
were causing any trouble, just doubled up "/" characters.
> I haven't combed through your patch in detail, so there might be similar
> problems lurking. I did notice one or two spots where you call
> strlen(PATH_REFS_*), which should of course also be changed to
> STRLEN_PATH_REFS_*.
I think I caught a few of them in my review. I think I had originally left
them like that on purpose. My reasoning was that a patch like that should
leave the resultant code identical. So I did replacements like:
- strlen("refs/heads/");
+ strlen(PATH_REFS_HEADS);
However, I think it was just pedantry, so I've been correcting them too.
I noticed a couple of places where memcmp() has been used where prefixcmp()
would work fine. I'm tempted to change them too - what do you think?
Perhaps a separate patch?
> And as a final comment, your patch doesn't apply to next at all because
> of the reorganization of the fetching API (e.g., fetch-pack.c doesn't
> exist at all anymore). You should probably prepare a parallel patch for
> next.
I'm happy to do prepare a patch against any revision, I was really waiting for
feedback from Junio as to how he'd like to manage it. Last time I submitted
this patch he (quite correctly) asked that I delay until after the next point
release; of course I promptly found other things to do and never
resubmitted :-)
Andy
--
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@xxxxxxxxx
-
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
