On Monday 2007 February 19 20:07, Junio C Hamano wrote:
> > - len = strlen(refs[i]) + 11;
> > + len = strlen(refs[i]) + STRLEN_PATH_REFS_TAGS + 1;
>
> shows that you've carefully looked at what the code does,
> instead of mindlessly replacing, which is a very good sign, but
> how much testing has this seen, I wonder.
Not huge quantities. The patch is so wide-spread, that I've checked it
compiles, and I've run with it for a bit. However, I could not swear to
having checked every path...
> > diff --git a/builtin-describe.c b/builtin-describe.c
> > index bcc6456..0f78363 100644
> > --- a/builtin-describe.c
> > +++ b/builtin-describe.c
> > @@ -52,7 +52,7 @@ static int get_name(const char *path, const unsigned
> > char *sha1, int flag, void * If --tags, then any tags are used.
> > * Otherwise only annotated tags are used.
> > */
> > - if (!strncmp(path, "refs/tags/", 10)) {
> > + if (!strncmp(path, PATH_TAGS, STRLEN_PATH_TAGS)) {
> > if (object->type == OBJ_TAG)
> > prio = 2;
> > else
>
> This is PATH_REFS_TAGS isn't it?
<shame> Yes.
> I mildly mind this one, as it hurts grep-ability. I know this is one
> of the the only two places in git that 'master' branch is treated
> specially (and I think we would like to keep it that way --- that's
The output of
$ git-grep "master" -- '*.c' '*.h'
Is not that big, so those special treatments can be easily found irrespective
of my patch.
> why I want to be able to grep for "refs/heads/master" and see very few
> hits), so introducing PATH_REFS_HEADS_MASTER is probably not very
> productive either, but... hmmmm.
As I've said before, I'm against /any/ special treatment of master other than
as a default branch name in a newly initialised repository.
PATH_REFS_HEADS_MASTER is closer to master not being special than without so
I'd be in favour of that.
> These repeated strncmp(p, X, STRLEN_X) almost makes me wonder if we
> want to introduce:
>
> inline int prefixcmp(a, b)
> {
> return (strncmp(a, b, strlen(b));
> }
>
> with clever preprocessor optimization to have compiler do strlen()
> when b is a string literal.
Wow; that would be clever - regardless of whether this patch is acceptable or
not.
Andy
--
Dr Andy Parkins, M Eng (hons), MIEE
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