Jeff King <peff@xxxxxxxx> 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 <peff@xxxxxxxx>
> ---
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)