Antoine Delaite <[email protected]> writes:

> -     if (!strcmp(refname, "bad")) {
> +     char good_prefix[256];
> +     strcpy(good_prefix, name_good);
> +     strcat(good_prefix, "-");

You are silently adding a restriction here: name_good must be small
enough to fit in a 256-bytes array. It's not a terrible restriction, but
what may happen if you break it is a real issue.

Either you have to enforce this restriction somewhere, or you should not
have the restriction at all. I'd vote for the second. strbuf is your
friend here.

> @@ -259,21 +264,21 @@ bisect_state() {
>  
>  bisect_next_check() {
>       missing_good= missing_bad=
> -     git show-ref -q --verify refs/bisect/bad || missing_bad=t
> -     test -n "$(git for-each-ref "refs/bisect/good-*")" || missing_good=t
> +     git show-ref -q --verify refs/bisect/$NAME_BAD || missing_bad=t
> +     test -n "$(git for-each-ref "refs/bisect/$NAME_GOOD-*")" || 
> missing_good=t

There are other restrictions here: $NAME_BAD must be an acceptable ref
name, and you're not quoting $NAME_BAD hence it must not contain shell
meta-characters (The requirements for ref names almost imply that, but
'foo/bar{a,b}' is accepted and will trigger some expansion if your
/bin/sh is bash for example).

Being an acceptable ref name is a constraint you have to check (Junio
already mentionned check-ref-format). I think quoting variables makes
sense too.

> @@ -421,7 +426,7 @@ bisect_replay () {
>               start)
>                       cmd="bisect_start $rev"
>                       eval "$cmd" ;;
> -             good|bad|skip)
> +             $NAME_GOOD|$NAME_BAD|skip)

$NAME_GOOD and $NAME_BAD need quoting if you're not sure they don't
contain shell metacharacters.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to