Re: [PATCH v2 9/9] sha1_name.c: get_describe_name() by definition groks only commits

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 21, 2012, 19:57 UTC
Message-ID: <7v1ul8sajk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CABPQNSZ246Y80WJN+u2OR8_3KWE_1=8F6WbTgVX-8wqyecGPkg@mail.gmail.com>

Erik Faye-Lund <kusmabite@gmail.com> writes:

Show 27 quoted lines

> On Thu, Jun 21, 2012 at 8:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  sha1_name.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/sha1_name.c b/sha1_name.c
>> index 58dbbe2..15e97eb 100644
>> --- a/sha1_name.c
>> +++ b/sha1_name.c
>> @@ -606,6 +606,7 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)
>>  static int get_describe_name(const char *name, int len, unsigned char *sha1)
>>  {
>>        const char *cp;
>> +       unsigned flags = GET_SHORT_QUIETLY | GET_SHORT_COMMIT_ONLY;
>>
>>        for (cp = name + len - 1; name + 2 <= cp; cp--) {
>>                char ch = *cp;
>> @@ -616,7 +617,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1)
>>                        if (ch == 'g' && cp[-1] == '-') {
>>                                cp++;
>>                                len -= cp - name;
>> -                               return get_short_sha1(cp, len, sha1, GET_SHORT_QUIETLY);
>> +                               return get_short_sha1(cp, len, sha1, flags);
>
> Is there a reason why you chose to put the definition in the
> root-scope of the function?

Mostly because it never changes inside the loop. "unsigned const flags" upfront might have even been a better option, though.

A less important reason is it would make the resulting file wider, not narrower, than the original.