Jeff King <p...@peff.net> writes:

> The get_sha1() family of functions takes a flags field, but
> some of the flags are mutually exclusive. In particular, we
> can only handle one disambiguating function, and the flags
> quietly override each other. Let's instead detect these as
> programming bugs.
>
> Technically some of the flags are supersets of the others,
> so treating COMMITTISH|TREEISH as just COMMITTISH is not
> wrong, but it's a good sign the caller is confused. And
> certainly asking for BLOB|TREE does not work.
>
> We can do the check easily with some bit-twiddling, and as a
> bonus, the bit-mask of disambiguators will come in handy in
> a future patch.
>
> Signed-off-by: Jeff King <p...@peff.net>
> ---

Other than your reinvention of HAS_MULTI_BITS(), which has been with
us since db7244bd ("parse-options new features.", 2007-11-07), this
looks like a reasonable thing to do.

;-)

>  cache.h     | 5 +++++
>  sha1_name.c | 9 +++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/cache.h b/cache.h
> index d0494c8..7bd78ca 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1203,6 +1203,11 @@ struct object_context {
>  #define GET_SHA1_FOLLOW_SYMLINKS 0100
>  #define GET_SHA1_ONLY_TO_DIE    04000
>  
> +#define GET_SHA1_DISAMBIGUATORS \
> +     (GET_SHA1_COMMIT | GET_SHA1_COMMITTISH | \
> +     GET_SHA1_TREE | GET_SHA1_TREEISH | \
> +     GET_SHA1_BLOB)
> +
>  extern int get_sha1(const char *str, unsigned char *sha1);
>  extern int get_sha1_commit(const char *str, unsigned char *sha1);
>  extern int get_sha1_committish(const char *str, unsigned char *sha1);
> diff --git a/sha1_name.c b/sha1_name.c
> index faf873c..f9812ff 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -310,6 +310,11 @@ static int prepare_prefixes(const char *name, int len,
>       return 0;
>  }
>  
> +static int multiple_bits_set(unsigned flags)
> +{
> +     return !!(flags & (flags - 1));
> +}
> +
>  static int get_short_sha1(const char *name, int len, unsigned char *sha1,
>                         unsigned flags)
>  {
> @@ -327,6 +332,10 @@ static int get_short_sha1(const char *name, int len, 
> unsigned char *sha1,
>       prepare_alt_odb();
>  
>       memset(&ds, 0, sizeof(ds));
> +
> +     if (multiple_bits_set(flags & GET_SHA1_DISAMBIGUATORS))
> +             die("BUG: multiple get_short_sha1 disambiguator flags");
> +
>       if (flags & GET_SHA1_COMMIT)
>               ds.fn = disambiguate_commit_only;
>       else if (flags & GET_SHA1_COMMITTISH)

Reply via email to