Joshua Juran venit, vidit, dixit 03.05.2011 10:17:
> On May 2, 2011, at 11:33 PM, Michael J Gruber wrote:
>
>> Joshua Juran venit, vidit, dixit 03.05.2011 03:57:
>>> On May 2, 2011, at 12:15 PM, Junio C Hamano wrote:
>>>
>>>> diff --git a/revision.c b/revision.c
>>>> index f4b8b38..26271d1 100644
>>>> --- a/revision.c
>>>> +++ b/revision.c
>>>> @@ -905,14 +905,26 @@ int handle_revision_arg(const char *arg,
>>>> struct rev_info *revs,
>>>> const char *this = arg;
>>>> int symmetric = *next == '.';
>>>> unsigned int flags_exclude = flags ^ UNINTERESTING;
>>>> + static const char head_by_default[] = "HEAD";
>>>>
>>>> *dotdot = 0;
>>>> next += symmetric;
>>>>
>>>> if (!*next)
>>>> - next = "HEAD";
>>>> + next = head_by_default;
>>>> if (dotdot == arg)
>>>> - this = "HEAD";
>>>> + this = head_by_default;
>>>> + if (this == head_by_default && next == head_by_default &&
>>>> + !symmetric) {
>>>
>>> Is there a reason not to write
>>>
>>> const char *head_by_default = "HEAD";
>>>
>>> or even
>>>
>>> const char *const head_by_default = "HEAD";
>>>
>>> instead? Loading a static array and checking an init flag is a
>>> pessimization versus just pointing into a read-only segment.
>>
>> Because of the comparisons later on: this == "HEAD" is not the same.
>
> Sorry if I was unclear. I meant to replace only the 'static const
> char head_by_default[] = "HEAD";' line and leave the rest of the patch
> unchanged.
>
> My recollection is that Metrowerks C will statically allocate read-
> write storage (duplicating the read-only copy of the string) and copy
> the string into it at runtime. It only copies the string once, which
> is ensured by checking an internally generated flag (also statically
> allocated) every time the scope containing the declaration is
> executed. This is the pessimization I speak of.
>
> By contrast, my suggestion allocates a single pointer on the stack
> regardless of compiler optimization.
I see, sorry, I misread your suggestion. Junio will have to answer for
himself then ;)
Michael
--
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